On Mon, Jan 29, 2024 at 8:03 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Mon, Jan 29, 2024 at 06:36:47AM -0800, H.J. Lu wrote:
> > TARGET_ASM_SELECT_RTX_SECTION is for constant in RTL.
> > It should have a non-public label reference which can't be used
> > by other TUs.  The same section can contain other constants.
> > If there is a COMAT issue, linker will catch it.
>
> Let me try to explain on short assembly snippet what I believe your patch is
> doing and what I'm afraid of.  I believe your patch when we need to emit
> a RTL constant foo or foo+1 or foo+2 (where foo is defined in a comdat
> section) instead of emitting using say foo in assembly puts those
> constants into .data.rel.ro.local section determined by the decl that is
> referenced.
> Now, when first_tu.o wins and emits the qux comdat, it will contain
> the .data.rel.ro.local.foo which bar function refers to, but in second_tu.o
> it wants to refer to different offsets from the same function and loses.
>
> I simply believe the constants need to be in section based on what refers
> to those symbols, not the value of those constants, and that is what we used
> to do before your patch (and I'd like to understand what's wrong with what
> GCC emits and why).
>
> first_tu.s:
> ============
>         .section        .text.foo,"axG",@progbits,qux,comdat
>         .p2align 4
>         .type   foo, @function
> foo:
>         xorl    %eax, %eax
>         ret
>         .size   foo, .-foo
>         .text
>         .p2align 4
>         .type   bar, @function
> bar:
>         movq    .LC0(%rip), %xmm0
>         ret
>         .size   bar, .-bar
>         .section        .data.rel.ro.local.foo,"awG",@progbits,qux,comdat
>         .align 8
> .LC0:
>         .quad   foo
>
> second_tu.s:
> ============
>         .section        .text.foo,"axG",@progbits,qux,comdat
>         .p2align 4
>         .type   foo, @function
> foo:
>         xorl    %eax, %eax
>         ret
>         .size   foo, .-foo
>         .text
>         .p2align 4
>         .type   baz, @function
> baz:
>         movq    .LC0(%rip), %xmm0
>         ret

I don't think this is valid.  We can't reference a non-public
symbol outside of a COMDAT group.  It is OK to reference
foo or foo + 1, but not .LC0.

>         .size   baz, .-baz
>         .section        .data.rel.ro.local.foo,"awG",@progbits,qux,comdat
>         .align 8
> .LC0:
>         .quad   foo+1
>         .text
>         .p2align 4
>         .type   corge, @function
> corge:
>         movq    .LC1(%rip), %xmm0
>         ret
>         .size   corge, .-corge
>         .section        .data.rel.ro.local.foo,"awG",@progbits,qux,comdat
>         .align 8
> .LC1:
>         .quad   foo+2
> gcc -shared -o test.so first_tu.s second_tu.s
> `.data.rel.ro.local.foo' referenced in section `.text' of /tmp/cceeUWyH.o: 
> defined in discarded section `.data.rel.ro.local.foo[qux]' of /tmp/cceeUWyH.o
> `.data.rel.ro.local.foo' referenced in section `.text' of /tmp/cceeUWyH.o: 
> defined in discarded section `.data.rel.ro.local.foo[qux]' of /tmp/cceeUWyH.o
> collect2: error: ld returned 1 exit status
>
>         Jakub
>


-- 
H.J.

Reply via email to