[PATCH] D116092: [XCOFF] make sure same number of paddings are added

Thu Jun 23 21:20:36 PDT 2022

shchenz marked 3 inline comments as done.
shchenz added a comment.

> I don't think a non-NFC patch should be missing tests, how about making D114419 <https://reviews.llvm.org/D114419> to the parent of this patch and leaving an error check there to be fixed in this patch?

A reasonable concern. I also tried to do this. But while I was adding `not` to the run command and checking `warning` or `error` in the output of objdump, I felt maybe it is not so good. Without this patch, if we commit D114419 <https://reviews.llvm.org/D114419> first, we certainly introduce regression although it will be fixed in this patch, but we still make some builds in bug state. So I think we should commit this patch first and then commit D114419 <https://reviews.llvm.org/D114419>. Adding an option to change the alignment for DWARF section in this file can let me cook some testcase for the change, but that also does not sound like a good idea.

Let me your thoughts. Thanks.

================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:1103
+    LastDwarfSection->MemorySize = Address - LastDwarfSection->Address;
   }

----------------
Esme wrote:
> Why do you want the last Dwarf section to be aligned with the DefaultSectionAlign? It seems to me that it is followed by relocation and not sections, right?
Right. No sections after the dwarf section. But it is better to align the final section as loading contents from aligned address is preferred. This also align with previous behavior.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116092