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

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.


================
Comment at: clang/include/clang/Basic/LangOptions.def:186
 LANGOPT(PPCIEEELongDouble            , 1, 0, "use IEEE 754 quadruple-precision for long double")
+LANGOPT(EnableAIXExtendedAltivecABI        , 1, 0, "__EXTABI__  predefined macro")
 COMPATIBLE_VALUE_LANGOPT(PICLevel    , 2, 0, "__PIC__ level")
----------------
minor nit: remove extra spaces in front of `1`.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1445
+          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)
----------------
I am wondering why do we add `!T.isOSBinFormatXCOFF()` back?


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1449
+
+    if (!Args.hasArg(OPT_maltivec))
+      Diags.Report(diag::err_aix_altivec);
----------------
Just a record here, as we discussed offline, we don't need to emit this error in frontend.


================
Comment at: clang/test/CodeGen/altivec.c:7
+ 
+// RUN: %clang -S -emit-llvm -maltivec -mabi=vec-extabi -target powerpc-unknown-aix %s -o - | FileCheck %s
+// RUN: not %clang -S -emit-llvm -mabi=vec-default -target powerpc-unknown-aix %s 2>&1  | FileCheck  %s --check-prefix=AIX-ATVER
----------------
When user specify `-maltivec / -target-feature +altivec`  without using any abi option,  the compiler will assume default altivec abi. In this situation, since default abi hasn’t been implemented, we should emit an error. So can we also add testcases for :

```
// RUN: not %clang -S -emit-llvm -maltivec -target powerpc-unknown-aix %s 2>&1 | FileCheck %s --check-prefix=AIX-ERROR
and
// RUN: not %clang_cc1 -target-feature +altivec -triple powerpc-unknown-aix -emit-llvm %s 2>&1 | FileCheck %s --check-prefix=AIX-ERROR
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89684