[PATCH] D65159: [PowerPC][XCOFF] Adds support for writing the .bss section for object files.

[Xing Xue via Phabricator via llvm-commits]

xingxue added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:44
 
+constexpr unsigned DefaultSectionAlign = 16;
+
----------------
hubert.reinterpretcast wrote:
> sfertile wrote:
> > hubert.reinterpretcast wrote:
> > > sfertile wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > My observations of the XLC and GCC behaviour on AIX does not support what this patch does with this value. Namely, it is observed that the physical/virtual address assigned to the `.bss` section is not necessarily 16-byte aligned.
> > > > I think we chose a default of 16 for all sections to keep it simple, after seeing that gcc aligned .data to 16 bytes, but other sections only to 4. 
> > > This is from a GCC-compiled object file; the `.data` section is not 16-byte aligned:
> > > ```
> > >                          Section Header for .data
> > > PHYaddr      VTRaddr     SCTsiz      RAWptr      RELptr
> > > 0x00000004  0x00000004  0x00000014  0x00000068  0x0000007c
> > > ```
> > > 
> > I'm thoroughly confused at this point. gcc emits assembly and for a bunch of different test cases and it consistently emits `.csect .data[RW], 4`. According the the AIX assembly language reference, the expression after the csect name and storage mapping class is the log base-2 of alignment. Even with this alignment, some tools report the .data sections alignment as 2^3, some as 16, and the virtual address assigned to it typically ends up being **only** 4-byte aligned as you observed. I'm probably mistaking the .data csect for the .data sections alignment in some of these cases, but even if the .data section contains a 16 byte aligned .data csect, why is it not 16 byte aligned? sigh.
> > 
> > The binary output  llvm writes, and the binary output the system assembler will produce for the assembly llvm produces from the same input should semantically match as close as possible. At this point I think choosing the correct long term behavior is outside the scope of this patch, whose intent to add //experimental //support. If an alignment of 16 on the .data csect  was chosen only for matching gcc's assembly output then I am fine changing both assembly output and binary output to some other default (4-byte ?) alignment . If there was some other important limitation causing us to choose 16-byte aligned for assembly output then I think we keep this patch as is, until we can find out if hte system assemblers alignment treatment is intentional or not.
> > 
> >  @xingxue I would appreciate your thoughts on relaxing the default alignment in the assembly output.
> I am not aware of a way to refer to the XCOFF sections in assembly. The belief that XCOFF sections are low-level artifacts below the level of assembly is one of the reasons why `MCSectionXCOFF` represents a CSECT and not a section.
> 
> The `.data[RW], 4` //does// modify the symbol table entry for the CSECT to indicate 16-byte alignment. CSECTs in object files are aligned in relation to the physical/virtual address allocation and not in relation to the base address of their containing section. The snippet I posted above is for a case where the `.data` section contains a 16-byte aligned CSECT at 0x10. The offset of the CSECT data in relation to the start of the data section image in the object file is 0xC, which is not a multiple of 16.
What Hubert says makes sense.  Variables in `.csect .data[RW]` are contained in the `.data` section where the alignments specified for `csect` are honored. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65159