[PATCH] D65159: [PowerPC][XCOFF] Adds support for writing the .bss section for object files.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast requested changes to this revision.
hubert.reinterpretcast added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:238
+    switch (MCSec->getMappingClass()) {
+    case XCOFF::XMC_PR: {
+      assert(XCOFF::XTY_SD == MCSec->getCSectType() &&
----------------
Why the braces for this case but not the others?


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:241
+             "Only an initilized csect can contain program code.");
+      Add(&Text, ProgramCodeCSects, MCSec);
+      break;
----------------
This code does not belong in this patch. The patch modifies `Text`, but does not add it to the list of non-empty sections.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:249
+      }
+      report_fatal_error("Unhandled mapping of read-write csect to section");
+    default:
----------------
Add a period to the end of the sentence.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:251
+    default:
+      report_fatal_error("Unhandled mapping of csect to section");
+    }
----------------
Same comment.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:262
+    // If the name does not fit in the storage provided in the symbol table
+    // entry add it to the string table.
+    if (XSym->getName().size() >= XCOFF::NameSize) {
----------------
Comma before "add".


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:263
+    // entry add it to the string table.
+    if (XSym->getName().size() >= XCOFF::NameSize) {
+      Strings.add(XSym->getName());
----------------
Off-by-one error. This can be fixed by using `nameInStringTable` on the `Symbol` created for the symbol.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:344
+    W.write<uint32_t>(0);
+    // Relocation and Linno counts. Not supported yet.
+    W.write<uint16_t>(0);
----------------
`Linno` is the abbreviation used for field in an auxiliary entry, not for the XCOFF section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65159