[PATCH] D61694: Boilerplate for producing XCOFF object files from the PowerPC backend.

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked 20 inline comments as done and an inline comment as not done.
sfertile added inline comments.


================
Comment at: llvm/include/llvm/MC/MCContext.h:260
+      bool operator<(const XCOFFSectionKey &Other) const {
+        if (SectionName != Other.SectionName)
+          return SectionName < Other.SectionName;
----------------
hubert.reinterpretcast wrote:
> I suggest trying:
> ```
>         return std::tie(SectionName, MappingClass) <
>                std::tie(Other.SectionName, Other.MappingClass);
> ```
> 
Good suggestion, thanks.


================
Comment at: llvm/lib/MC/MCSectionXCOFF.cpp:22
+  if (getKind().isText()) {
+    OS << "\t.csect " << getSectionName() << "["
+       << "PR"
----------------
hubert.reinterpretcast wrote:
> Is there a test for this?
There is now :)


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:57
+  // We always emit a timestamp of 0 for reporducibility, so ensure incremental
+  // linking is not enabled.
+  if (Asm.isIncrementalLinkerCompatible())
----------------
hubert.reinterpretcast wrote:
> How is an XCOFF timestamp associated with "incremental linking" (apparently a feature of `link.exe`)?
`IncrementalLinkerCompatible` is a field in the generic MCAssembler and I don't see anything to indicate that its limited to Windows. The  fact that toolchains that target 'Link.exe' are the only one to enabled it currently doesn't mean that won't change in the future. I'd rather be pedantic and show that there might be some compatibility issues between the behavior we have chosen for XCOFF and the MCAssembler behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61694