[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/lib/MC/MCSectionXCOFF.cpp:22
+  if (getKind().isText()) {
+    OS << "\t.csect " << getSectionName() << "["
+       << "PR"
----------------
Is there a test for this?


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:26
+
+  virtual void executePostLayoutBinding(MCAssembler &,
+                                        const MCAsmLayout &) override;
----------------
The prevailing style in the containing directory is not to have both `virtual` and `override` on the same declaration. This comment applies to multiple instances in this file.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:56
+uint64_t XCOFFObjectWriter::writeObject(MCAssembler &Asm, const MCAsmLayout &) {
+  // We always emit a timestamp of 0 for reporducibility, so ensure incremental
+  // linking is not enabled.
----------------
Typo: s/reporducibility/reproducibility/;


================
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())
----------------
How is an XCOFF timestamp associated with "incremental linking" (apparently a feature of `link.exe`)?


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:86
+
+} // end anonymous namespace.
+
----------------
No period after "namespace".


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.cpp:86
+
+PPCXCOFFMCAsmInfo::PPCXCOFFMCAsmInfo(bool is64Bit, const Triple &T) {
+  assert(!IsLittleEndian && "Little endian XCOFF not supported.");
----------------
s/is64Bit/Is64Bit/;


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCAsmInfo.cpp:87
+PPCXCOFFMCAsmInfo::PPCXCOFFMCAsmInfo(bool is64Bit, const Triple &T) {
+  assert(!IsLittleEndian && "Little endian XCOFF not supported.");
+  CodePointerSize = CalleeSaveStackSlotSize = is64Bit ? 8 : 4;
----------------
"Little-endian" with the hyphen.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:245
+  void emitTCEntry(const MCSymbol &S) override {
+    report_fatal_error("TOC entry not supported yet.");
+  }
----------------
"TOC entries are not supported yet."


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:249
+  void emitMachine(StringRef CPU) override {
+    llvm_unreachable("emit machine pseduo-op no tsupported on XCOFF.");
+  }
----------------
Typos. Suggestion (if indeed the restriction is inherent):
"Invalid request to emit a machine pseudo-op for XCOFF."



================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCXCOFFObjectWriter.cpp:20
+};
+} // end anonymous namespace.
+
----------------
Remove the period.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61694