Testsuite: Add btf-dataset option for RISC-V.

Message ID 20211230030201.842664-1-jiawei@iscas.ac.cn
State Deferred, archived
Headers
Series Testsuite: Add btf-dataset option for RISC-V. |

Commit Message

Jiawei Dec. 30, 2021, 3:02 a.m. UTC
  Add -msmall-data-limit option to put global and static data into right
section and generate 'btt_info' on RISC-V target.

BTF (BPF Type Format) is the metadata format which encodes the debug info related to BPF program/map, more details on:
https://www.kernel.org/doc/html/latest/bpf/index.html#bpf-type-format-btf

gcc/testsuite/ChangeLog:

        * gcc.dg/debug/btf/btf-datasec-1.c: Add riscv target support.

---
 gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Jeff Law Dec. 30, 2021, 4:28 p.m. UTC | #1
On 12/29/2021 8:02 PM, jiawei wrote:
> Add -msmall-data-limit option to put global and static data into right
> section and generate 'btt_info' on RISC-V target.
>
> BTF (BPF Type Format) is the metadata format which encodes the debug info related to BPF program/map, more details on:
> https://www.kernel.org/doc/html/latest/bpf/index.html#bpf-type-format-btf
>
> gcc/testsuite/ChangeLog:
>
>          * gcc.dg/debug/btf/btf-datasec-1.c: Add riscv target support.
Is the goal here to get the variable "d" out of the small data section 
and into the standard data section?  It's not clear from your description .

Neither an ACK nor a NAK at this point.  I need to understand better 
what you're trying to accomplish.

jeff
  
Palmer Dabbelt Dec. 30, 2021, 4:59 p.m. UTC | #2
On Thu, 30 Dec 2021 08:28:34 PST (-0800), gcc-patches@gcc.gnu.org wrote:
>
>
> On 12/29/2021 8:02 PM, jiawei wrote:
>> Add -msmall-data-limit option to put global and static data into right
>> section and generate 'btt_info' on RISC-V target.
>>
>> BTF (BPF Type Format) is the metadata format which encodes the debug info related to BPF program/map, more details on:
>> https://www.kernel.org/doc/html/latest/bpf/index.html#bpf-type-format-btf
>>
>> gcc/testsuite/ChangeLog:
>>
>>          * gcc.dg/debug/btf/btf-datasec-1.c: Add riscv target support.
> Is the goal here to get the variable "d" out of the small data section
> and into the standard data section?  It's not clear from your description .
>
> Neither an ACK nor a NAK at this point.  I need to understand better
> what you're trying to accomplish.

IIUC that's what this is doing, though the commit message isn't clear at 
all.  That saind, it might be better to do something like

    diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
    index dbb236bbda1..c0ad77d40aa 100644
    --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
    +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
    @@ -22,9 +22,9 @@
     /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 7 } } */
    
     /* Check that strings for each DATASEC have been added to the BTF string table.  */
    -/* { dg-final { scan-assembler-times "ascii \".data.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
    -/* { dg-final { scan-assembler-times "ascii \".rodata.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
    -/* { dg-final { scan-assembler-times "ascii \".bss.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
    +/* { dg-final { scan-assembler-times "ascii \".[s]?data.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
    +/* { dg-final { scan-assembler-times "ascii \".[s]?rodata.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
    +/* { dg-final { scan-assembler-times "ascii \".[s]?bss.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
    
     int a;
     long long b;

as whether specific symbols end up in .data or .sdata is really just an 
optimization and either should be sufficiently correct.

IIRC some targets have other flavors of these, PPC's .sdata2 comes to 
mind.  Not sure if we'd need that to drop their -msdata=none flag.
  
Jiawei Dec. 31, 2021, 2:29 a.m. UTC | #3
> -----原始邮件-----
&gt; 发件人: "Palmer Dabbelt" <palmer@rivosinc.com>
&gt; 发送时间: 2021-12-31 00:59:08 (星期五)
&gt; 收件人: jeffreyalaw@gmail.com
&gt; 抄送: gcc-patches@gcc.gnu.org, jiawei@iscas.ac.cn, gcc-patches@gcc.gnu.org, indu.bhagat@oracle.com, kito.cheng@sifive.com, yulong@nj.iscas.ac.cn, sinan@isrc.iscas.ac.cn, shihua@iscas.ac.cn
&gt; 主题: Re: [PATCH] Testsuite: Add btf-dataset option for RISC-V.
&gt; 
&gt; On Thu, 30 Dec 2021 08:28:34 PST (-0800), gcc-patches@gcc.gnu.org wrote:
&gt; &gt;
&gt; &gt;
&gt; &gt; On 12/29/2021 8:02 PM, jiawei wrote:
&gt; &gt;&gt; Add -msmall-data-limit option to put global and static data into right
&gt; &gt;&gt; section and generate 'btt_info' on RISC-V target.
&gt; &gt;&gt;
&gt; &gt;&gt; BTF (BPF Type Format) is the metadata format which encodes the debug info related to BPF program/map, more details on:
&gt; &gt;&gt; https://www.kernel.org/doc/html/latest/bpf/index.html#bpf-type-format-btf
&gt; &gt;&gt;
&gt; &gt;&gt; gcc/testsuite/ChangeLog:
&gt; &gt;&gt;
&gt; &gt;&gt;          * gcc.dg/debug/btf/btf-datasec-1.c: Add riscv target support.
&gt; &gt; Is the goal here to get the variable "d" out of the small data section
&gt; &gt; and into the standard data section?&nbsp; It's not clear from your description .
&gt; &gt;
&gt; &gt; Neither an ACK nor a NAK at this point.&nbsp; I need to understand better
&gt; &gt; what you're trying to accomplish.


Thanks for your mentions, the testcase said it expect 3 DATASEC records: one for each of .data, .rodata and .bss.
If we only use /* { dg-options "-O0 -gbtf -dA" } */ as default compile option on riscv target like:

```````````````````
$ riscv64-unknown-elf-gcc -S -O0 -gbtf -dA btf-datasec-1.c 
```````````````````

the variable in section will be throw into .sdata, .sbss, .srodata and don't fit the check require, and I add the additional option '-msmall-data-limit' to fix it, to get a correct .section part, we need to limit the '-msmall-data-limit' less than the smallest variable 'int a' size define as 4 byte, then it will be send into section .bss.

```````````````````
$ riscv64-unknown-elf-gcc -S -O0 -gbtf -dA btf-datasec-1.c -o btf-datasec-1.s

.Ltext0:
        .cfi_sections   .debug_frame
        .file 0 "/root/test" "btf-datasec-1.c"
        .globl  a
        .section        .sbss,"aw",@nobits
        .align  2
        .type   a, @object
        .size   a, 4

$ riscv64-unknown-elf-gcc -S -O0 -gbtf -dA -msmall-data-limit=2 btf-datasec-1.c -o btf-datasec-2.s

.Ltext0:
        .cfi_sections   .debug_frame
        .file 0 "/root/test" "btf-datasec-1.c"
        .globl  a
        .bss
        .align  2
        .type   a, @object
        .size   a, 4
```````````````````

It is seem on other variable, and when all section set right, all check in the testcase can pass on riscv.

```````````````````
$ diff btf-datasec-1.s btf-datasec-2.s

&gt;       .bss
144c163
&lt;       .section        .srodata,"a"
---
&gt;       .section        .rodata
151c170
&lt;       .section        .sdata,"aw"
---
&gt;       .data
158c177
&lt;       .section        .srodata
---
&gt;       .section        .rodata
```````````````````

&gt; 
&gt; IIUC that's what this is doing, though the commit message isn't clear at 
&gt; all.  That saind, it might be better to do something like


Sorry for the unclear commit, I explained the idea in this mail, should I resend the patch?


&gt; 
&gt;     diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
&gt;     index dbb236bbda1..c0ad77d40aa 100644
&gt;     --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
&gt;     +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
&gt;     @@ -22,9 +22,9 @@
&gt;      /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 7 } } */
&gt;     
&gt;      /* Check that strings for each DATASEC have been added to the BTF string table.  */
&gt;     -/* { dg-final { scan-assembler-times "ascii \".data.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
&gt;     -/* { dg-final { scan-assembler-times "ascii \".rodata.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
&gt;     -/* { dg-final { scan-assembler-times "ascii \".bss.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
&gt;     +/* { dg-final { scan-assembler-times "ascii \".[s]?data.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
&gt;     +/* { dg-final { scan-assembler-times "ascii \".[s]?rodata.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
&gt;     +/* { dg-final { scan-assembler-times "ascii \".[s]?bss.0\"\[\t \]+\[^\n\]*btf_aux_string" 1 } } */
&gt;     
&gt;      int a;
&gt;      long long b;
&gt; 
&gt; as whether specific symbols end up in .data or .sdata is really just an 
&gt; optimization and either should be sufficiently correct.
&gt; 


Yes, I think you are quite right, this is a question that the symbol goes different section after optimization.


&gt; IIRC some targets have other flavors of these, PPC's .sdata2 comes to 
&gt; mind.  Not sure if we'd need that to drop their -msdata=none flag.

I'm not familiar with other target, maybe there are some error parts with my understanding, Thanks at all!


</palmer@rivosinc.com>
  
Indu Bhagat Jan. 3, 2022, 4:43 p.m. UTC | #4
On 12/30/21 8:59 AM, Palmer Dabbelt wrote:
> On Thu, 30 Dec 2021 08:28:34 PST (-0800), gcc-patches@gcc.gnu.org wrote:
>>
>>
>> On 12/29/2021 8:02 PM, jiawei wrote:
>>> Add -msmall-data-limit option to put global and static data into right
>>> section and generate 'btt_info' on RISC-V target.
>>>
>>> BTF (BPF Type Format) is the metadata format which encodes the debug 
>>> info related to BPF program/map, more details on:
>>> https://www.kernel.org/doc/html/latest/bpf/index.html#bpf-type-format-btf 
>>>
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>          * gcc.dg/debug/btf/btf-datasec-1.c: Add riscv target support.
>> Is the goal here to get the variable "d" out of the small data section
>> and into the standard data section?  It's not clear from your 
>> description .
>>
>> Neither an ACK nor a NAK at this point.  I need to understand better
>> what you're trying to accomplish.
> 
> IIUC that's what this is doing, though the commit message isn't clear at 
> all.  That saind, it might be better to do something like
> 
>     diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c 
> b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>     index dbb236bbda1..c0ad77d40aa 100644
>     --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>     +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>     @@ -22,9 +22,9 @@
>      /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 
> 7 } } */
>      /* Check that strings for each DATASEC have been added to the BTF 
> string table.  */
>     -/* { dg-final { scan-assembler-times "ascii \".data.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>     -/* { dg-final { scan-assembler-times "ascii \".rodata.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>     -/* { dg-final { scan-assembler-times "ascii \".bss.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>     +/* { dg-final { scan-assembler-times "ascii \".[s]?data.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>     +/* { dg-final { scan-assembler-times "ascii \".[s]?rodata.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>     +/* { dg-final { scan-assembler-times "ascii \".[s]?bss.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>      int a;
>      long long b;
> 
> as whether specific symbols end up in .data or .sdata is really just an 
> optimization and either should be sufficiently correct.

Yes, I would agree that the test case can be adapted as mentioned. The 
purpose of the test case is to check that BTF is correctly generated for 
whatever section the symbols end up in.

Adding David Faust in CC for ACK.

Thanks
Indu

> 
> IIRC some targets have other flavors of these, PPC's .sdata2 comes to 
> mind.  Not sure if we'd need that to drop their -msdata=none flag.
  
Jeff Law Jan. 3, 2022, 4:56 p.m. UTC | #5
On 12/30/2021 9:59 AM, Palmer Dabbelt wrote:
> On Thu, 30 Dec 2021 08:28:34 PST (-0800), gcc-patches@gcc.gnu.org wrote:
>>
>>
>> On 12/29/2021 8:02 PM, jiawei wrote:
>>> Add -msmall-data-limit option to put global and static data into right
>>> section and generate 'btt_info' on RISC-V target.
>>>
>>> BTF (BPF Type Format) is the metadata format which encodes the debug 
>>> info related to BPF program/map, more details on:
>>> https://www.kernel.org/doc/html/latest/bpf/index.html#bpf-type-format-btf 
>>>
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>          * gcc.dg/debug/btf/btf-datasec-1.c: Add riscv target support.
>> Is the goal here to get the variable "d" out of the small data section
>> and into the standard data section?  It's not clear from your 
>> description .
>>
>> Neither an ACK nor a NAK at this point.  I need to understand better
>> what you're trying to accomplish.
>
> IIUC that's what this is doing, though the commit message isn't clear 
> at all.  That saind, it might be better to do something like
It might.  My only real hesitation would be if where was some wonky BTF 
dependency on having the symbols in the normal sections.  But I'd 
consider that unlikey.
>
>    diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c 
> b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>    index dbb236bbda1..c0ad77d40aa 100644
>    --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>    +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>    @@ -22,9 +22,9 @@
>     /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 
> 7 } } */
>        /* Check that strings for each DATASEC have been added to the 
> BTF string table.  */
>    -/* { dg-final { scan-assembler-times "ascii \".data.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>    -/* { dg-final { scan-assembler-times "ascii \".rodata.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>    -/* { dg-final { scan-assembler-times "ascii \".bss.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>    +/* { dg-final { scan-assembler-times "ascii \".[s]?data.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>    +/* { dg-final { scan-assembler-times "ascii \".[s]?rodata.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>    +/* { dg-final { scan-assembler-times "ascii \".[s]?bss.0\"\[\t 
> \]+\[^\n\]*btf_aux_string" 1 } } */
>        int a;
>     long long b;
>
> as whether specific symbols end up in .data or .sdata is really just 
> an optimization and either should be sufficiently correct.
Probably missing some escapes in your variant (the brackets). Jiawei, 
could you try Palmer suggestion to verify it works for you?

>
> IIRC some targets have other flavors of these, PPC's .sdata2 comes to 
> mind.  Not sure if we'd need that to drop their -msdata=none flag.
I'd expect them to continue to work as-is.  We could potentially drop 
those flags as a separate patch after suitable testing.

jeff
  
David Faust Jan. 3, 2022, 5:36 p.m. UTC | #6
On 1/3/22 08:43, Indu Bhagat wrote:
> On 12/30/21 8:59 AM, Palmer Dabbelt wrote:
>> On Thu, 30 Dec 2021 08:28:34 PST (-0800), gcc-patches@gcc.gnu.org wrote:
>>>
>>>
>>> On 12/29/2021 8:02 PM, jiawei wrote:
>>>> Add -msmall-data-limit option to put global and static data into right
>>>> section and generate 'btt_info' on RISC-V target.
>>>>
>>>> BTF (BPF Type Format) is the metadata format which encodes the debug
>>>> info related to BPF program/map, more details on:
>>>> https://www.kernel.org/doc/html/latest/bpf/index.html#bpf-type-format-btf
>>>>
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>           * gcc.dg/debug/btf/btf-datasec-1.c: Add riscv target support.
>>> Is the goal here to get the variable "d" out of the small data section
>>> and into the standard data section?  It's not clear from your
>>> description .
>>>
>>> Neither an ACK nor a NAK at this point.  I need to understand better
>>> what you're trying to accomplish.
>>
>> IIUC that's what this is doing, though the commit message isn't clear at
>> all.  That saind, it might be better to do something like
>>
>>      diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>> b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>>      index dbb236bbda1..c0ad77d40aa 100644
>>      --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>>      +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>>      @@ -22,9 +22,9 @@
>>       /* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset"
>> 7 } } */
>>       /* Check that strings for each DATASEC have been added to the BTF
>> string table.  */
>>      -/* { dg-final { scan-assembler-times "ascii \".data.0\"\[\t
>> \]+\[^\n\]*btf_aux_string" 1 } } */
>>      -/* { dg-final { scan-assembler-times "ascii \".rodata.0\"\[\t
>> \]+\[^\n\]*btf_aux_string" 1 } } */
>>      -/* { dg-final { scan-assembler-times "ascii \".bss.0\"\[\t
>> \]+\[^\n\]*btf_aux_string" 1 } } */
>>      +/* { dg-final { scan-assembler-times "ascii \".[s]?data.0\"\[\t
>> \]+\[^\n\]*btf_aux_string" 1 } } */
>>      +/* { dg-final { scan-assembler-times "ascii \".[s]?rodata.0\"\[\t
>> \]+\[^\n\]*btf_aux_string" 1 } } */
>>      +/* { dg-final { scan-assembler-times "ascii \".[s]?bss.0\"\[\t
>> \]+\[^\n\]*btf_aux_string" 1 } } */
>>       int a;
>>       long long b;
>>
>> as whether specific symbols end up in .data or .sdata is really just an
>> optimization and either should be sufficiently correct.
> 
> Yes, I would agree that the test case can be adapted as mentioned. The
> purpose of the test case is to check that BTF is correctly generated for
> whatever section the symbols end up in.
> 
> Adding David Faust in CC for ACK.

Thanks Indu

I concur. This looks like a good improvement to me. If any of the 
existing target-specific options in the test could eventually be dropped 
as a result, then all the better IMO.

David

> 
> Thanks
> Indu
> 
>>
>> IIRC some targets have other flavors of these, PPC's .sdata2 comes to
>> mind.  Not sure if we'd need that to drop their -msdata=none flag.
>
  

Patch

diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
index dbb236bbda1..d54c8b1f63b 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
@@ -13,6 +13,7 @@ 
 /* { dg-options "-O0 -gbtf -dA" } */
 /* { dg-options "-O0 -gbtf -dA -msdata=none" { target { { powerpc*-*-* } && ilp32 } } } */
 /* { dg-options "-O0 -gbtf -dA -G0" { target { nios2-*-* } } } */
+/* { dg-options "-O0 -gbtf -dA -msmall-data-limit=2" { target { riscv*-*-* } } } */
 
 /* Check for two DATASEC entries with vlen 3, and one with vlen 1.  */
 /* { dg-final { scan-assembler-times "0xf000003\[\t \]+\[^\n\]*btt_info" 2 } } */