[PATCH] D65159: [PowerPC][XCOFF] Adds support for writing the .bss section for object files.
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 18 22:16:19 PDT 2019
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
More information about the llvm-commits
mailing list