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.