[PATCH] [gdb.fortran] Add type info of formal parameter for clang in ptype-on-functions.exp

Kumar N, Bhuvanendra Bhuvanendra.KumarN@amd.com
Fri Apr 30 08:52:51 GMT 2021


[AMD Official Use Only - Internal Distribution Only]

Hi Andrew and all,

Gentle Ping!

Regards,
bhuvan

-----Original Message-----
From: Kumar N, Bhuvanendra
Sent: Thursday, April 22, 2021 12:19 PM
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: gdb-patches@sourceware.org; George, Jini Susan <JiniSusan.George@amd.com>; Achra, Nitika <Nitika.Achra@amd.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; E, Nagajyothi <Nagajyothi.E@amd.com>; Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>
Subject: RE: [PATCH] [gdb.fortran] Add type info of formal parameter for clang in ptype-on-functions.exp

[AMD Official Use Only - Internal Distribution Only]

Hi Andrew,

Thanks for your valuable comments, multiple issues are there, mostly during the test patch inlining, encoding went wrong. Hence I have attached the relevant info or files as attachments this time. Hope its fine.

> I didn't really understand what you were trying to do in this commit 
> message, nor do I see how commit 0a709cba00d relates to this change

I have attached the git show and git log command output as attachment here (commit id: 0a709cba00d36d490482d0e8673e323ac1e897a6). As explained in the commit message, basically the test case expects the kind information as integer(kind=4) or integer(kind=8). But flang by default emits kind information as integer or integer*8 respectively, hence there is mismatch in GDB command output comparison. This required test case modification and it was done earlier as mentioned in this commit message by Alok CC'ed in this email.
Hence similar "kind" related changes were done in ptype-on-functions.exp.

> This formatting here is wrong, it should be

Thanks, I have corrected Changelog now

> I'm not sure what happened with the encoding of this email (note 'file://')...

I have attached the test patch and also the test case itself for better clarity here, inlining was messed up earlier and its confusing, could you please refer the attachments, please

> I'm not suggesting it's wrong, but I've really curious what the '10' here represents.  I don't see the value 10 anywhere in the test source code, and as this is a type signature, I wouldn't really expect a value in this position.

As we can see in the below flang emitted IR(intermediate representation), flang compiler is emitting additional formal parameter of type array with 10 elements. Flang or any other compiler creates these type of additional and internal variables or parameters for its purpose. I am covering the type information of these flang emitted internal parameters in this test case change, also addressing the default "kind" information emitted by flang. These type of changes for flang was done in other test cases also, earlier.

flang emitted IR snippet: 

define signext i32 @some_module_get_number_(i64* noalias %this, i64* noalias %.O0000) #0 !dbg !26 { . . .
        call void @llvm.dbg.declare (metadata i64* %this, metadata !31, metadata !20), !dbg !27
        call void @llvm.dbg.declare (metadata i64* %.O0000, metadata !36, metadata !20), !dbg !27 . . .
!32 = !DISubrange(lowerBound: 1, upperBound: 10)
!33 = !DIBasicType(tag: DW_TAG_base_type, name: "integer*8", size: 64, align: 64, encoding: DW_ATE_signed)
!34 = !{ !32 }
!35 = !DICompositeType(tag: DW_TAG_array_type, size: 640, align: 64, baseType: !33, elements: !34)
!36 = !DILocalVariable(scope: !26, arg: 2, file: !3, line: 28, type: !35, flags: 64)

> In general, rather than forking two separate code paths, could we not 
> try to extract the differences out into variables

I have attached the modified test case also,  gfortran and flang differs with i.number of formal parameters ii.type of each of these formal parameters, hence for different compiler, followed different code path

Thanks again for your review, please let me know any other details if required

Regards,
bhuvan

-----Original Message-----
From: Andrew Burgess <andrew.burgess@embecosm.com>
Sent: Wednesday, April 21, 2021 9:43 PM
To: Kumar N, Bhuvanendra <Bhuvanendra.KumarN@amd.com>
Cc: gdb-patches@sourceware.org; George, Jini Susan <JiniSusan.George@amd.com>; Achra, Nitika <Nitika.Achra@amd.com>; Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; E, Nagajyothi <Nagajyothi.E@amd.com>; Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>
Subject: Re: [PATCH] [gdb.fortran] Add type info of formal parameter for clang in ptype-on-functions.exp

[CAUTION: External Email]

* Kumar N, Bhuvanendra via Gdb-patches <gdb-patches@sourceware.org> [2021-04-15 21:37:05 +0000]:

Thanks for working on this.

> Additional compiler generated formal parameter exist with clang and 
> type information for the same is added accordingly. Also few kind 
> parameter printing are removed which is not default for clang.
> Note: More details about this kind parameter omission while printing 
> can be found with similar patch
>   commit 0a709cba00d36d490482d0e8673e323ac1e897a6
>   Author Alok Kumar Sharma
> (alokkumar.sharma@amd.com<mailto:alokkumar.sharma@amd.com>)

I didn't really understand what you were trying to so in this commit message, nor do I see how commit 0a709cba00d relates to this change.

>
> gdb/testsuite/ChangeLog:
>       * gdb.fortran/ptype-on-functions.exp: Add type info of formal
>       parameter for clang. Also removed the kind parameter for clang.
>       * lib/fortran.exp (fortran_int8): Likewise.
> ---
> gdb/testsuite/ChangeLog                       |  5 ++
> .../gdb.fortran/ptype-on-functions.exp        | 55 ++++++++++++++-----
> gdb/testsuite/lib/fortran.exp                 |  2 +-
> 3 files changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index
> ad289c135d..216d985995 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2021-04-16  Bhuvanendra Kumar  Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
> +     * gdb.fortran/ptype-on-functions.exp: Add type info of formal
> +     parameter for clang. Also removed the kind parameter for clang.
> +     * lib/fortran.exp (fortran_int8): Likewise.

This formatting here is wrong, it should be:

  2021-04-16  Bhuvanendra Kumar  <Bhuvanendra.KumarN@amd.com>

        * gdb.fortran/ptype-on-functions.exp: Add type info of formal
        parameter for clang. Also removed the kind parameter for clang.
        * lib/fortran.exp (fortran_int8): Likewise.

> +
> 2021-04-16  Bhuvanendra Kumar  Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
>          * gdb.base/foll-exec.exp: Additional next commands added for 
> diff --git a/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
> b/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
> index 14f522d6d4..4e46e10166 100644
> --- a/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
> +++ b/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
> @@ -29,23 +29,50 @@ if ![fortran_runto_main] then {
>      continue
> }
> -gdb_test "ptype some_module::get_number" \
> -    "type = integer\\(kind=4\\) \\(Type<file://(Type> __class_some_module_Number(_t)?\\)"

I'm not sure what happened with the encoding of this email (note 'file://')...

> +if {[test_compiler_info {clang-*}]} {
> +    set integer4 [fortran_int4]
> +    set logical4 [fortran_logical4]
> +    set integer8 [fortran_int8]
> -gdb_test "ptype some_module::set_number" \
> -    "type = void \\(Type<file://(Type> __class_some_module_Number(_t)?, integer\\(kind=4\\)\\)"
> +    gdb_test "ptype some_module::get_number" \
> +        "type = $integer4 \\(Type<file://(Type> number, $integer8 \\(10\\)\\<file://(10/)/>)"

I'm not suggesting it's wrong, but I've really curious what the '10'
here represents.  I don't see the value 10 anywhere in the test source code, and as this is a type signature, I wouldn't really expect a value in this position.

In general, rather than forking two separate code paths, could we not try to extract the differences out into variables, so we might have something like:

  gdb_test "ptype some_module::get_number" \
      "type = $integer4 \\($number_class_type\\)"

doing things this way helps prevent tests from diverging too much for the different compilers.

Thanks,
Andrew

> -gdb_test "ptype is_bigger" \
> -    "type = logical\\(kind=4\\) \\(integer\\(kind=4\\<file://(integer/(kind=4/>), integer\\(kind=4\\)\\)"
> +    gdb_test "ptype some_module::set_number" \
> +        "type = void \\(Type<file://(Type> number, $integer4\\, $integer8 \\(10\\)\\<file://(10/)/>)"
> -gdb_test "ptype say_numbers" \
> -    "type = void \\(integer\\(kind=4\\<file://(integer/(kind=4/>), integer\\(kind=4\\), integer\\(kind=4\\)\\)"
> +    gdb_test "ptype is_bigger" \
> +        "type = $logical4 \\($integer4<file://($integer4>, $integer4\\)"
> -gdb_test "ptype fun_ptr" \
> -    "type = PTR TO -> \\( integer\\(kind=4\\) \\(\\) \\(REF<file://(REF> TO -> \\( integer\\(kind=4\\) \\)\\) \\)"
> +    gdb_test "ptype say_numbers" \
> +        "type = void \\($integer4<file://($integer4>, $integer4, $integer4\\)"
> -gdb_test "ptype say_string" \
> -    "type = void \\(character\\*\\(\\*\\<file://(character/*/(/*/>), integer\\(kind=\\d+\\)\\)"
> +    gdb_test "ptype fun_ptr" \
> +        "type = PTR TO -> \\( $integer4 \\(\\) \\($integer4\\)<file://($integer4/)> \\)"
> -gdb_test "ptype say_array" \
> -    "type = void \\(integer\\(kind=4\\<file://(integer/(kind=4/>) \\(:,:\\)\\)"
> +    gdb_test "ptype say_string" \
> +        "type = void \\(character\\*\\(\\*\\<file://(character/*/(/*/>), $integer8\\)"
> +
> +    gdb_test "ptype say_array" \
> +        "type = void \\($integer8<file://($integer8>, $integer4 \\(:,:\\)\\)"
> +} else {
> +    gdb_test "ptype some_module::get_number" \
> +        "type = integer\\(kind=4\\) \\(Type<file://(Type> __class_some_module_Number(_t)?\\)"
> +
> +    gdb_test "ptype some_module::set_number" \
> +        "type = void \\(Type<file://(Type> __class_some_module_Number(_t)?, integer\\(kind=4\\)\\)"
> +
> +    gdb_test "ptype is_bigger" \
> +        "type = logical\\(kind=4\\) \\(integer\\(kind=4\\<file://(integer/(kind=4/>), integer\\(kind=4\\)\\)"
> +
> +    gdb_test "ptype say_numbers" \
> +        "type = void \\(integer\\(kind=4\\<file://(integer/(kind=4/>), integer\\(kind=4\\), integer\\(kind=4\\)\\)"
> +
> +    gdb_test "ptype fun_ptr" \
> +        "type = PTR TO -> \\( integer\\(kind=4\\) \\(\\) \\(REF<file://(REF> TO -> \\( integer\\(kind=4\\) \\)\\) \\)"
> +
> +    gdb_test "ptype say_string" \
> +        "type = void \\(character\\*\\(\\*\\<file://(character/*/(/*/>), integer\\(kind=\\d+\\)\\)"
> +
> +    gdb_test "ptype say_array" \
> +        "type = void \\(integer\\(kind=4\\<file://(integer/(kind=4/>) \\(:,:\\)\\)"
> +}
> diff --git a/gdb/testsuite/lib/fortran.exp 
> b/gdb/testsuite/lib/fortran.exp index 35db863e9c..f8c442fd09 100644
> --- a/gdb/testsuite/lib/fortran.exp
> +++ b/gdb/testsuite/lib/fortran.exp
> @@ -49,7 +49,7 @@ proc fortran_int8 {} {
>      } elseif {[test_compiler_info {gcc-*}]} {
>      return "integer\\(kind=8\\)"
>      } elseif {[test_compiler_info {clang-*}]} {
> -     return "integer*8"
> +     return "integer\\*8"
>      } elseif {[test_compiler_info {icc-*}]} {
>      return "INTEGER\\(8\\)"
>      } else {
> --
> 2.17.1
>


More information about the Gdb-patches mailing list