[PATCH] D104639: [AIX][XCOFF] Add support for 64-bit file header and section header writing

[Sean Fertile via Phabricator via llvm-commits]

sfertile added a comment.

In D104639#2832518 <https://reviews.llvm.org/D104639#2832518>, @jhenderson wrote:

> As a non-XCOFF developer, this mostly looks fine to me, but needs a test case.

This is important, we can't approve and land a patch without testing, and this patch is currently in an untestable state as we will hit a fatal error before hitting the code changes in writeFileHeader/writeSectionHeaderTable. On the converse side, there is nothing to stop object file writing after emitting the file header and section header tables.

I've left some inline comments on this patch showing where we will have to make adjustments for your changes to be testable. There is one other modification we need to make in `PPCXCOFFObjectWriter::getRelocTypeAndSignSize` which gets called as we finalize the layout. We need to add support for mapping the `FK_Data_8` fixup type with a VK_None modifier (all the other modifiers should be left supported for now). Note that since the last field is the signdness and (1 - the length) and we are relocating 8 byte data now it will differ slightly from the existing relocations. Something like

  case FK_Data_8:
    switch (Modifier) {
    default:
      report_fatal_error("Unsupported modifier");
    case MCSymbolRefExpr::VK_None:
      return {XCOFF::RelocationType::R_POS, EncodedSignednessIndicator | 63};
    }

should be fine for this patch, and we can handle the other modifiers in the follow up patches.



================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:30
 constexpr size_t FileHeaderSize32 = 20;
+constexpr size_t FileHeaderSize64 = 24;
 constexpr size_t SectionHeaderSize32 = 40;
----------------
I'm not sure why these changes are showing up in this patch as they are all landed upstream already (they landed in the patch adding support for yaml2obj support for xcoff)


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:347
                                                  const MCAsmLayout &Layout) {
   if (TargetObjectWriter->is64Bit())
     report_fatal_error("64-bit XCOFF object files are not supported yet.");
----------------
This fatal error needs to be removed to be able to reach the point where we emit the file-header and section header table.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:359
     // entry, add it to the string table.
     if (nameShouldBeInStringTable(MCSec->getSymbolTableName()))
       Strings.add(MCSec->getSymbolTableName());
----------------
The implementation of this function will need to be modified to be correct for 64-bit object generation since the name is always stored in the string table (or in a debug section) and not in the symbol table entry itself.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:562
 
   if (TargetObjectWriter->is64Bit())
     report_fatal_error("64-bit XCOFF object files are not supported yet.");
----------------
This fatal error will also need to be removed to be able to observe your changes to `writeFileHeader` and `writeSectionheaderTable`


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:569
   writeFileHeader();
   writeSectionHeaderTable();
   writeSections(Asm, Layout);
----------------
We should insert an early return after this line, with a comment saying this is the temporary end of the 64-bit implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104639