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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:33
+  XMC_GL = 6,      ///< Global Linkage (Interfile Interface Code)
+  XMC_XO = 7,      ///< Extended Operation (Pseudo Machine Instruction
+  XMC_SV = 8,      ///< Supervisor Call (32-bit process only)
----------------
Missing close paren.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:36
+  XMC_SV64 = 17,   ///< Supervisor Call for 64-bit process
+  XMC_SV3264 = 18, ///< Supervisor Call for both 32- and 64-bit processes.
+  XMC_TI = 12,     ///< Traceback Index csect
----------------
No period to be consistent with the other comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:38
+  XMC_TI = 12,     ///< Traceback Index csect
+  XMC_TB = 13,     ///< Traceback table csect
+
----------------
Uppercase "table" to be consistent with the other comments.


================
Comment at: llvm/include/llvm/MC/MCContext.h:254
+      std::string SectionName;
+      unsigned MappingClass;
+
----------------
Why `unsigned` here, and `XCOFF::StorageMappingClass` elsewhere?


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



================
Comment at: llvm/include/llvm/MC/MCXCOFFObjectWriter.h:23
+public:
+  virtual ~MCXCOFFObjectTargetWriter();
+
----------------
Use `override`.


================
Comment at: llvm/include/llvm/MC/MCXCOFFObjectWriter.h:25
+
+  virtual Triple::ObjectFormatType getFormat() const { return Triple::XCOFF; }
+  static bool classof(const MCObjectTargetWriter *W) {
----------------
Use `override`.


================
Comment at: llvm/include/llvm/MC/MCXCOFFObjectWriter.h:34
+                        raw_pwrite_stream &OS);
+} // namespace llvm
+
----------------
Most instances in the containing directory have a newline before the closing brace and "end namespace llvm".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61694