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

[Digger via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:89
   uint32_t Size;
+  uint32_t PaddingSize;
   uint32_t FileOffsetToData;
----------------
hubert.reinterpretcast wrote:
> Remove this field (see comments on later lines).
After I discuss with Sean, We decide to keep variable PaddingSize here. with additional variable,  it can make the code more readable and the logic of padding more easy to understand. 


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:103
     Size = 0;
+    PaddingSize = 0;
     FileOffsetToData = 0;
----------------
hubert.reinterpretcast wrote:
> Remove this field (see comments on later lines).
After I discuss with Sean, We decide to keep variable PaddingSize here. with additional variable, it can make the code more readable and the logic of padding more easy to understand.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:511
+      const MCSectionXCOFF *MCSec = Csect.MCCsect;
+      Csect.PaddingSize = alignTo(Address, MCSec->getAlignment()) - Address;
+      Address += Csect.PaddingSize;
----------------
hubert.reinterpretcast wrote:
> The inter-csect padding is not really a property of the csect requiring alignment. We do not need to store this value here. The amount of padding to write can be determined by tracking the virtual address of the raw section data being written during the serialization into the object file. The next virtual address following the padding should be `Csect.Address`.
I got what you talk about, But  If we do not calculate the padding size here, We have to calculate the padding size in the XCOFFObjectWriter::writeSections , it will make the logic of the function writeSections  complicated. 

for example  
 // Write the program code control sections one at a time.
  uint32_t PaddingSize = 0;   //additional variable here.
  for (auto it= ProgramCodeCsects.begin(); it!=ProgramCodeCsects.end() ;++it ) {
    if (PaddingSize)
      W.OS.write_zeros(Csect.PaddingSize);
   // And I think I also need some comment to explain following code.
   if(std::next(it) != ProgramCodeCsects.end() )
       PaddingSize = std::next(it)->Address - it->Address - it->size;
    Asm.writeSectionData(W.OS, Csect.MCCsect, Layout);
  }



================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:525
+    }
+    Text.PaddingSize = alignTo(Address, DefaultSectionAlign) - Address;
+    Address += Text.PaddingSize;
----------------
hubert.reinterpretcast wrote:
> The `Size` field accounts for the padding. We do not need to store this value here. The amount of padding to write can be determined by tracking the virtual address of the raw section data being written during the serialization into the object file. The next virtual address following the padding should be `Text.Address + Text.Size`.
some reason as above.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66969