[PATCH] D79127: [XCOFF][AIX] Emit correct alignment for csect

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:58
   Symbol->setCommon(Size, ByteAlignment);
+  // Default csect align is 4, but common symbols have explicit alignment value
+  // and we should honor it.
----------------
Suggest newline before the comment.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1717
+  // TODO: We might want to skip this early passing of setAlignment when data
+  // sections are implemented.
+  if (TM.getDataSections())
----------------
Even if data sections are used, explicitly-specified sections could be used to group variables together.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1735
+
+  // We need to know csect's alignment upfront for assembly path,
+  // because once .csect directive gets emitted, we could not change the
----------------
s/know csect's alignment upfront/know, up front, the alignment of csects/;
Add "the" before "assembly path".


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1736
+  // We need to know csect's alignment upfront for assembly path,
+  // because once .csect directive gets emitted, we could not change the
+  // alignment value on it.
----------------
Add "a" before ".csect directive".


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1741
+
+  return AsmPrinter::doFinalization(M);
+}
----------------
Use the immediate base class even if it does not currently override this function.


================
Comment at: llvm/test/CodeGen/PowerPC/test_func_desc.ll:6
 ; RUN: FileCheck --check-prefixes=CHECK,64BIT %s
 
 
----------------
It seems we have nothing to verify alignment of functions:
```
__attribute__((__aligned__(32))) int bar() { return 0; }
```


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

https://reviews.llvm.org/D79127