[PATCH] D66969: Output XCOFF object text section header and symbol entry for program code

[Digger via Phabricator via llvm-commits]

DiggerLin marked 7 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:391
+  // table entries for a detailed description. Bits 0-3 is for symbol
+  // visibility. Bit 10 (0x0020) can be set if the symbol is a function in the
+  // old XCOFF32 interpretation. Otherwise bit 10 should be 0. Since we don't
----------------
hubert.reinterpretcast wrote:
> DiggerLin wrote:
> > hubert.reinterpretcast wrote:
> > > Use the shorter version of the comment here too,
> > changed as suggestion
> There are some cases here of two consecutive spaces. It is not necessarily wrong, but we should be consistent.
delete extras space.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:298
+    Asm.writeSectionData(W.OS, Csect.MCCsect, Layout);
+
+}
----------------
hubert.reinterpretcast wrote:
> We have code that introduces alignment padding, so I suspect we should have code here to write out the padding.
Added new function to support it.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:525
     BSS.Index = SectionIndex++;
-    assert(alignTo(Address, DefaultSectionAlign) == Address &&
-           "Improperly aligned address for section.");
-    uint32_t StartAddress = Address;
+    BSS.Address = Address;
     for (auto &Csect : BSSCsects) {
----------------
hubert.reinterpretcast wrote:
> A comment is necessary here regarding alignment padding between the start of the section and the first item allocated within the section (which might require a stricter alignment). A question is why such padding is done "physically" in within the section as opposed to doing the padding "virtually" by adjusting the section's address in the address space.
added a comment


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll:84
 ; SYMS-NEXT:   Symbol {
-; SYMS-NEXT:     Index: [[#Index:]]
-; SYMS-NEXT:     Name: a
+; SYMS:          Index: [[#Index:]]{{[[:space:]].*}}Name: a
 ; SYMS-NEXT:     Value (RelocatableAddress): 0x0
----------------
hubert.reinterpretcast wrote:
> I think this warrants more explanation. Is the ability of the regex to match non-space characters intended?
there is     Index: 0
    Name: .text

before   Symbol {
    Index: 2
    Name: a

origin test case will match .text symbol first and the value of Index will be zero.

using ; SYMS:          Index: [[#Index:]]{{[[:space:]].*}}Name: a
can make sure that it match  Symbol {
    Index: 2
    Name: a 
not symbol .text and the index value will 2. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66969