[PATCH] D89684: [AIX] Add mabi=vec-extabi options to enable the AIX extended and default vector ABIs.

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1443
 
+  if (Arg *A =
+          Args.getLastArg(OPT_mabi_EQ_vec_default, OPT_mabi_EQ_vec_extabi)) {
----------------
ZarkoCA wrote:
> Xiangling_L wrote:
> > Should we also check if target feature altivec[`-target-feature +altivec`] is enabled when using these two options? If so, we should also add related testcases.
> Both of these options require that -maltivec is also selected which sets `-target-feature +altivec`.
> Both of these options require that -maltivec is also selected which sets `-target-feature +altivec`.

It seems the scenario you pointed out are driver invocation use case. But when users invoke clang -cc1 not driver with -mabi=ext-abi/default, should we also check if the user specify `-target-feature +altivec` as well? For now, based on my observation, if invoking cc1 without `-target-feature +altivec` but with `-mabi=ext-abi` only like `// RUN: %clang_cc1 -mabi=vec-extabi -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s`, the error is `error: unknown type name 'vector'`. Should we explicitly say `-target-feature +altivec` is required as what we do for driver?


================
Comment at: clang/test/Driver/aix-vec-extabi.c:2
+// RUN:  %clang -### -target powerpc-unknown-aix -S -maltivec -mabi=vec-extabi %s 2>&1 | \
+// RUN:  FileCheck %s --check-prefix=CHECK
+
----------------
minor: We can omit `--check-prefix=CHECK`


================
Comment at: llvm/include/llvm/Target/TargetOptions.h:179
 
+    /// AIXExtendedAltivecABI - This flag returns true when -vec-extabi is
+    /// specified. The code generator is then able to use both volatile and
----------------
minor: 
s/AIXExtendedAltivecABI/EnableAIXExtendedAltivecABI


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

https://reviews.llvm.org/D89684