[PATCH] D69620: Add AIX assembler support

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:37
+
+  // Specify the mode in which the as command operates.
+  if (IsArch32Bit) {
----------------
Refer to the other comment regarding confusion of "as" with the English word.


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:47
+  // displayed. Otherwise, undefined symbols are flagged with error messages.
+  CmdArgs.push_back("-u");
+
----------------
There is no FIXME here indicating the plan to remove this in the future when the assembly generation from the compiler no longer needs this.


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:49
+
+  // Acccept any mixture of instructions.
+  CmdArgs.push_back("-many");
----------------
Minor nit: Typo: s/Acccept/Accept/;


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:50
+  // Acccept any mixture of instructions.
+  CmdArgs.push_back("-many");
+
----------------
A comment should indicate that this behaviour matches that of GCC on Power for AIX and Linux for both the user-provided assembler source case and the compiler-produced assembler source case. The behaviour for XL with user-provided assemble source is different.


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:61
+
+  // Specify assembler input file(s).
+  for (const auto &II : Inputs)
----------------
The AIX assembler does not accept more than one input file. If the intent is to let the assembler produce the error message, then I think we should at least have a comment to acknowledge the situation.


================
Comment at: clang/test/Driver/aix-as.c:1
+// General tests that as invocations on AIX targets are sane. Note that we
+// only test assembler functionalities in this suite.
----------------
Minor nit: To avoid confusion with the English word, use
```
`as`
```
or
```
as(1)
```


================
Comment at: clang/test/Driver/aix-as.c:31
+// RUN:         -target powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS32-PTHREAD %s
+// CHECK-AS32-PTHREAD-NOT: warning:
----------------
I am not getting how this block is related to "pthread".


================
Comment at: clang/test/Driver/aix-as.c:40
+
+// Check powerpc-ibm-aix7.1.0.0, 64-bit. -Wa,<arg>,<arg> option.
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
----------------
s/powerpc/powerpc64/;


================
Comment at: clang/test/Driver/aix-as.c:44
+// RUN:         -target powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS64-PTHREAD %s
+// CHECK-AS64-PTHREAD-NOT: warning:
----------------
Same comment re: "pthread".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69620