[PATCH] D88339: [XCOFF] Enable -fdata-sections on AIX

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1992
           ->getQualNameSymbol();
-    if (GOKind.isCommon() || GOKind.isBSSLocal())
+    if (GOKind.isCommon() || GOKind.isBSSLocal() || TM.getDataSections())
       return cast<MCSectionXCOFF>(SectionForGlobal(GO, GOKind, TM))
----------------
daltenty wrote:
> We're currently gating this behavior on the code-gen data-sections option, but AIX common initialization does really function correctly without it, so we should probably adopt this as a default.
> 
> Summarizing from off-list discussion:
> 
> Variables with common linkage will be emitted as a common-type csect in XCOFF. Per the assembly reference: 
> 
> //"Several modules can share the same common block. If any of those modules have an external Control Section (csect) with the same name and the csect with the same name has a storage mapping class other than BS or UC, then the common block is initialized and becomes that other Control Section.”//
> 
> Initialization via a non-csect (i.e label-ed data) is not defined, and results in a linker warning:
> ```
> $ cat common.s
> .comm foo, 2
> $ cat label.s
> .csect data[RW]
> .globl foo
> foo:
>         .vbyte 2,0
> __start:
>         nop
> .globl __start
> $ ld common.o label.o
> ld: 0711-224 WARNING: Duplicate symbol: foo
> ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
> ```
Summarizing discussion offline:
Instead of making it a default and disallow user to specify -fno-data-sections on AIX, it's still worth providing user that option to turn it off when they know what they are doing and they think the linker is behaivoring correctly in their case.
But we should make the llc option -data-sections=true on AIX by default instead, so that we get the correct behaivor out of the box from llc. And for that, I will post a separate patch for it. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88339/new/

https://reviews.llvm.org/D88339