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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM with minor nits. Thanks.



================
Comment at: llvm/include/llvm/MC/MCSectionXCOFF.h:51
     QualName->setRepresentedCsect(this);
+    // Csect is 4 byte aligned by default, except for undefined symbol.
+    if (Type != XCOFF::XTY_ER)
----------------
s/Csect/A csect/;
Add "csects" after "undefined symbol".


================
Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:59
 
+  // Default csect align is 4, but common symbols have explicit alignment value
+  // and we should honor it.
----------------
s/value/values/;


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1716
+bool PPCAIXAsmPrinter::doInitialization(Module &M) {
+  const bool result = PPCAsmPrinter::doInitialization(M);
+
----------------
LLVM naming convention: `Result`.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-readonly-with-relocation.ll:8
 ;CHECK:         .comm   a[RW],4,2
-;CHECK-NEXT:    .csect .data[RW]
+;CHECK-NEXT:    .csect .data[RW], 2
 ;CHECK-NEXT:    .globl  b
----------------
jasonliu wrote:
> hubert.reinterpretcast wrote:
> > Should the style be consistent between `.csect` and `.comm` over the use of spaces after the comma?
> I agree the style should be consistent for the use of spaces after comma. 
> It seems like for clang, we would have a space after comma (at least for instructions). So looks like .csect directive is following that convention.
> We could have a NFC patch later to clean it up for .comm.
Okay; thanks.


================
Comment at: llvm/test/CodeGen/aix-func-align.ll:1
+; This test tries to verify if the csect contains code would have the correct alignment.
+
----------------
s/the csect contains/a csect containing/;


================
Comment at: llvm/test/CodeGen/aix-func-align.ll:10
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff -filetype=obj < %s 2>&1 | \
+; RUN: FileCheck --check-prefix=XCOFF64 %s
+; XCOFF64: LLVM ERROR: 64-bit XCOFF object files are not supported yet.
----------------
Indent the `FileCheck` invocation. From D78929, it looks like two spaces.


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

https://reviews.llvm.org/D79127