Xiangling_L added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4616
+ if (Args.hasArg(options::OPT_maltivec) &&
+ (Triple.isOSAIX() || Triple.isOSBinFormatXCOFF())) {
+ for (const Arg *A : Args) {
----------------
I am wondering what cases are not covered by `Triple.isOSAIX()`? Why do we also
query `Triple.isOSBinFormatXCOFF()`?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4617
+ (Triple.isOSAIX() || Triple.isOSBinFormatXCOFF())) {
+ for (const Arg *A : Args) {
+ auto optID = A->getOption().getID();
----------------
I see your intention here is to find if there are any `-mabi=vec-extabi` or
`-mabi=vec-default` option specifying. If I am correct, why we need to loop
through whole Args? Can we just use `hasArg` query?
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1417
+ if (Arg *A = Args.getLastArg(OPT_mabi_EQ_vec_default,
OPT_mabi_EQ_vec_extabi)) {
+ if (!T.isOSAIX() || !T.isOSBinFormatXCOFF())
+ Diags.Report(diag::err_drv_unsupported_opt_for_target)
----------------
ditto.
================
Comment at: clang/test/CodeGen/altivec.c:2
// RUN: %clang_cc1 -target-feature +altivec -triple powerpc-unknown-unknown
-emit-llvm %s -o - | FileCheck %s
-
+// RUN: %clang_cc1 -target-feature +altivec -mabi=vec-extabi -triple
powerpc-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -target-feature +altivec -mabi=vec-extabi -triple
powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
----------------
Based on the code added in `BackendUtil:551`, should we also add a case for
compiling a source to assembly?
================
Comment at: llvm/include/llvm/Target/TargetMachine.h:246
+ bool getAIXExtendedAltivecABI() const {
+ return Options.AIXExtendedAltivecABI;
----------------
I don't see this function gets invoked anywhere,can we remove it?
================
Comment at: llvm/include/llvm/Target/TargetOptions.h:179
+ /// AIXExtendedAltivecABI - This flag returns true when -mabi=vec-extabi is
+ /// specified. The code generator is then able to use both volatile and
----------------
-mabi=vec-extabi is the FE option, should we s/-mabi=vec-extabi/-vec-extabi?
================
Comment at: llvm/lib/CodeGen/CommandFlags.cpp:286
+ static cl::opt<bool> AIXExtendedAltivecABI(
+ "vec-extabi", cl::desc("Enable the AIX Extended Altivec ABI."),
----------------
Based on the naming convention in this file context, this seems should be
`EnableAIXExtendedAltivecABI`.
================
Comment at: llvm/lib/CodeGen/CommandFlags.cpp:287
+ static cl::opt<bool> AIXExtendedAltivecABI(
+ "vec-extabi", cl::desc("Enable the AIX Extended Altivec ABI."),
+ cl::init(false));
----------------
Can we add a testcase for this backend option?
================
Comment at: llvm/test/CodeGen/PowerPC/aix-default-vec-abi.ll:1
+; RUN: not --crash llc < %s -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr8 2>&1 |
FileCheck %s
+; RUN: not --crash llc < %s -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr8 2>&1 |
FileCheck %s
----------------
Can we simplify the testcase to only contain one vector type parameter? I think
that would be sufficient to trigger the error.
================
Comment at: llvm/test/CodeGen/PowerPC/aix-default-vec-abi.ll:4
+
+define dso_local <4 x i32> @vec_callee(<4 x i32> %vec1, <4 x i32> %vec2, <4 x
i32> %vec3, <4 x i32> %vec4, <4 x i32> %vec5, <4 x i32> %vec6, <4 x i32> %vec7,
<4 x i32> %vec8, <4 x i32> %vec9, <4 x i32> %vec10, <4 x i32> %vec11, <4 x i32>
%vec12, <4 x i32> %vec13, <4 x i32> %vec14) {
+ entry:
----------------
ditto: can we simplify the testcase?
================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll:7
-; RUN: llc -verify-machineinstrs -mcpu=pwr4 \
-; RUN: -mtriple powerpc-ibm-aix-xcoff -data-sections=false < %s |
FileCheck %s
-; RUN: llc -verify-machineinstrs -mcpu=pwr4 \
----------------
I am wondering why do we remove `-data-sections=false` here?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89684/new/
https://reviews.llvm.org/D89684
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
