[PATCH] D122287: [XCOFF] support writing sections, relocations and symbols for XCOFF64.

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:715
+    W.OS.write_zeros(6); // Reserved
+  }
 }
----------------
maybe this is more readable , even there is duplication code of    writeWord(NumberOfRelocEnt);


```
if (!is64Bit()) {
     W.OS.write_zeros(4); // Reserved
     writeWord(NumberOfRelocEnt);
     W.OS.write_zeros(6); // Reserved
  else {
     writeWord(NumberOfRelocEnt);
     W.OS.write_zeros(1); // Reserved
     W.write<uint8_t>(XCOFF::AUX_SECT);
  } 
```


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:196
   uint64_t RelocationEntryOffset = 0;
+  StringRef SourceFileName = ".file";
 
----------------
Esme wrote:
> DiggerLin wrote:
> >  I do not think  we will change  ".file" .  change to const StringRef SourceFileName = ".file" ?
> > If the file auxiliary entry is not used, the symbol name is the name of the source file. If the file auxiliary entry is used, then the symbol name should be .file, and the first file auxiliary entry (by convention) contains the source file name. 
> 
> Our current approach is to write a symbol without the auxiliary entry, with .file as the source file's name, but in fact we should write a symbol with the real source file's name. And we commented a todo for this.
> 
> ```
> // FIXME: add the real source file's name.
> ```
> So I think the SourceFileName may be changed.
thanks.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:751
   } else {
     W.write<int32_t>(SymbolTableEntryCount);
     W.write<uint16_t>(0); // AuxHeaderSize. No optional header for an object
----------------
Esme wrote:
> DiggerLin wrote:
> > SymbolTableEntryCount+1 for ".file" symbol.
> ```
> void XCOFFObjectWriter::assignAddressesAndIndices(const MCAsmLayout &Layout) {
>   // The first symbol table entry (at index 0) is for the file name.
>   uint32_t SymbolTableIndex = 1;
> ...
>   SymbolTableEntryCount = SymbolTableIndex;
> }
> ```
> The ".file" symbol has been counted.
thanks. 


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCXCOFFObjectWriter.cpp:82
+    case MCSymbolRefExpr::VK_None:
+      return {XCOFF::RelocationType::R_TOC, 15};
+    case MCSymbolRefExpr::VK_PPC_L:
----------------
Esme wrote:
> DiggerLin wrote:
> > I am not sure, whether I understand correct or not ?
> > 1. fixup_ppc_half16ds: A 14-bit fixup corresponding to lo16(_foo) with implied 2 zero bits for instrs like 'std'.
> > 2. fixup_ppc_half16dq:  A 16-bit fixup corresponding to lo16(_foo) with implied 3 zero bits for instrs like 'lxv'.
> > 3. fixup_ppc_half16 : A 16-bit fixup corresponding to lo16(_foo) or ha16(_foo) for instrs like 'li' or 'addis'.
> > 
> > in the https://www.ibm.com/docs/en/aix/7.2?topic=formats-xcoff-object-file-format#XCOFF__sua3i125jbau 
> > r_rsize: 0x3F(6 bits)
> > Specifies the bit length of the relocatable reference **minus one**. The current architecture allows for fields of up to 32 bits (XCOFF32) or 64 bits (XCOFF64) to be relocated.
> > 
> > so for the
> > case PPC::fixup_ppc_half16ds: 
> >     {XCOFF::RelocationType::R_TOC, EncodedSignednessIndicator | 13 } 
> Thanks for your catch!
PPC::fixup_ppc_half16dq: is a A 16-bit fixup corresponding to lo16(_foo) with implied 3 zero bits for instrs like 'lxv'. 
for  PPC::fixup_ppc_half16dq 
it should be 
{XCOFF::RelocationType::R_TOC, EncodedSignednessIndicator | 15 } ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122287