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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:35
 void MCXCOFFStreamer::EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
-                                       unsigned ByteAlignment) {
-  report_fatal_error("Emiting common symbols not implemented for XCOFF.");
+                                      unsigned ByteAlignment) {
+  getAssembler().registerSymbol(*Symbol);
----------------
Please revert the accidental whitespace change.


================
Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:37
+  getAssembler().registerSymbol(*Symbol);
+  Symbol->setExternal((cast<MCSymbolXCOFF>(Symbol))->getStorageClass() !=
+                      XCOFF::C_HIDEXT);
----------------
I recommend removing the extra parentheses.


================
Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:48
+  // Emit the alignment and storage for the variable to the section.
+  EmitValueToAlignment(ByteAlignment, 0, 1, 0);
+  EmitZeros(Size);
----------------
I would recommend picking up the default argument values instead of specifying them explicitly as magic numbers with no context.


================
Comment at: llvm/lib/MC/StringTableBuilder.cpp:72
+  // The COFF formats store the size of the string table in the first 4 bytes.
+  // For Windows, the format is little-endian; for AIX it is big-endian.
+  if (K == WinCOFF)
----------------
Comma after "AIX".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65159