Hi Jakub,
Thanks for the patch and appologies for the results regression.
Cupertino
Jakub Jelinek writes:
> On Tue, Mar 26, 2024 at 02:28:52PM +0000, Cupertino Miranda wrote:
>> GCC was defining bts_offset entry to always contain 0.
>> When comparing with clang, the same entry is instead a label to the
>> respective variable or function. The assembler emits relocations for
>> those labels.
>>
>> gcc/ChangeLog:
>> PR target/114431
>> * btfout.cc (get_name_for_datasec_entry): Add function.
>> (btf_asm_datasec_entry): Print label when possible.
>>
>> gcc/testsuite/ChangeLog:
>> gcc.dg/debug/btf/btf-datasec-1.c: Correct for new
>> implementation.
>> gcc.dg/debug/btf/btf-datasec-2.c: Likewise
>> gcc.dg/debug/btf/btf-pr106773.c: Likewise
>
> For next time, missing dot after Likewise
>
>> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
>> @@ -19,8 +19,10 @@
>> /* { dg-final { scan-assembler-times "0xf000003\[\t \]+\[^\n\]*btt_info" 2
>> } } */
>> /* { dg-final { scan-assembler-times "0xf000001\[\t \]+\[^\n\]*btt_info" 1
>> } } */
>>
>> -/* The offset entry for each variable in a DATSEC should be 0 at compile
>> time. */
>> -/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 7 } } */
>> +/* The offset entry for each variable in a DATSEC should contain a label.
>> */
>> +/* { dg-final { scan-assembler-times ".4byte\[\t \]\[a-e\]\[\t
>> \]+\[^\n\]*bts_offset" 5 } } */
>
> Where has this been tested?
> 4byte is used only on some targets, what exact assembler directive is used
> for 4byte unaligned data is heavily target dependent.
> git grep define.*TARGET_ASM_UNALIGNED_SI_OP
> gcc/config/alpha/alpha.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\t.align
> 0\n\t.long\t"
> gcc/config/avr/avr.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\t.long\t"
> gcc/config/cris/cris.cc:#define TARGET_ASM_UNALIGNED_SI_OP
> TARGET_ASM_ALIGNED_SI_OP
> gcc/config/csky/csky.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\t.long\t"
> gcc/config/i386/i386.cc:#define TARGET_ASM_UNALIGNED_SI_OP
> TARGET_ASM_ALIGNED_SI_OP
> gcc/config/ia64/ia64.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\tdata4.ua\t"
> gcc/config/m68k/m68k.cc:#define TARGET_ASM_UNALIGNED_SI_OP
> TARGET_ASM_ALIGNED_SI_OP
> gcc/config/mcore/mcore.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\t.long\t"
> gcc/config/pa/pa.cc:#define TARGET_ASM_UNALIGNED_SI_OP
> TARGET_ASM_ALIGNED_SI_OP
> gcc/config/rs6000/rs6000.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\t.vbyte\t4,"
> gcc/config/rs6000/rs6000.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\t.long\t"
> gcc/config/sh/sh.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\t.ualong\t"
> gcc/config/sparc/sparc.cc:#define TARGET_ASM_UNALIGNED_SI_OP "\t.uaword\t"
> gcc/target-def.h:#define TARGET_ASM_UNALIGNED_SI_OP "\t.4byte\t"
> gcc/target-def.h:#define TARGET_ASM_UNALIGNED_SI_OP NULL
> Now
> git grep 'define[[:blank:]]*TARGET_ASM_ALIGNED_SI_OP' | grep
> '/\(cris\|i386\|m68k\|pa\)/'
> gcc/config/cris/cris.cc:#define TARGET_ASM_ALIGNED_SI_OP "\t.dword\t"
> gcc/config/i386/i386.cc:#define TARGET_ASM_ALIGNED_SI_OP ASM_LONG
> gcc/config/m68k/m68k.cc:#define TARGET_ASM_ALIGNED_SI_OP "\tlong\t"
> gcc/config/m68k/m68k.cc:#define TARGET_ASM_ALIGNED_SI_OP "\tdc.l\t"
> gcc/config/pa/pa.cc:#define TARGET_ASM_ALIGNED_SI_OP "\t.word\t"
> git grep 'define[[:blank:]]*ASM_LONG'
> gcc/config/i386/att.h:#define ASM_LONG "\t.long\t"
> gcc/config/i386/bsd.h:#define ASM_LONG "\t.long\t"
> gcc/config/i386/darwin.h:#define ASM_LONG "\t.long\t"
>
> So, if you want to handle it for all arches, instead of .4byte\[\t \] you'd
> need to
> use
> (?:(?:.4byte|.long|data4.ua|.ualong|.uaword|.dword|long|dc.l|.word)\[\t
> \]|.vbyte\t4,\[\t \]?)
> or so.
>
> BTW,
> /* { dg-options "-O0 -gbtf -dA" } */
> /* { dg-options "-O0 -gbtf -dA -msdata=none" { target { { powerpc*-*-* } &&
> ilp32 } } } */
> /* { dg-options "-O0 -gbtf -dA -msmall-data-limit=0" { target { riscv*-*-* }
> } } */
> /* { dg-options "-O0 -gbtf -dA -G0" { target { nios2-*-* } } } */
> would be perhaps right for test written 2 decades ago, but for a test
> written 3 years ago is something that shouldn't have passed patch review.
> This should be done with
> /* { dg-options "-O0 -gbtf -dA" } */
> /* { dg-additional-options "-msdata=none" { target { { powerpc*-*-* } &&
> ilp32 } } } */
> /* { dg-additional-options "-msmall-data-limit=0" { target { riscv*-*-* } } }
> */
> /* { dg-additional-options "-G0" { target { nios2-*-* } } } */
>
> btf-cvr-quals-1.c test suffers from that too.
>
> Jakub
