Re: [PATCH] Testsuite: Add btf-dataset option for RISC-V.

2022-01-03 Thread David Faust via Gcc-patches




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.




Re: [PATCH] Testsuite: Add btf-dataset option for RISC-V.

2022-01-03 Thread Jeff Law via Gcc-patches




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



Re: [PATCH] Testsuite: Add btf-dataset option for RISC-V.

2022-01-03 Thread Indu Bhagat via Gcc-patches

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.




Re: Re: [PATCH] Testsuite: Add btf-dataset option for RISC-V.

2021-12-30 Thread 陈嘉炜



 -原始邮件-
 发件人: "Palmer Dabbelt" 
 发送时间: 2021-12-31 00:59:08 (星期五)
 收件人: jeffreya...@gmail.com
 抄送: gcc-patches@gcc.gnu.org, jia...@iscas.ac.cn, gcc-patches@gcc.gnu.org, 
indu.bha...@oracle.com, kito.ch...@sifive.com, yul...@nj.iscas.ac.cn, 
si...@isrc.iscas.ac.cn, shi...@iscas.ac.cn
 主题: Re: [PATCH] Testsuite: Add btf-dataset option for RISC-V.
 
 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.


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

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

 
 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


Sorry for the unclear commit, I explained the idea in this mail, should I 
resend the 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..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 think you are quite right, this is a question that the symbol goes 
different section after optimization.


 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'm not familiar with other target, maybe there are some error parts with my 
understanding, Thanks at all!




Re: [PATCH] Testsuite: Add btf-dataset option for RISC-V.

2021-12-30 Thread Palmer Dabbelt

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.


Re: [PATCH] Testsuite: Add btf-dataset option for RISC-V.

2021-12-30 Thread Jeff Law via Gcc-patches




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



[PATCH] Testsuite: Add btf-dataset option for RISC-V.

2021-12-29 Thread jiawei
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(+)

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 "0xf03\[\t \]+\[^\n\]*btt_info" 2 } } 
*/
-- 
2.25.1