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

Zarko Todorovski via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 19 12:51:00 PST 2020


ZarkoCA marked 10 inline comments as done.
ZarkoCA 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) {
----------------
Xiangling_L wrote:
> I am wondering what cases are not covered by `Triple.isOSAIX()`? Why do we also query `Triple.isOSBinFormatXCOFF()`?
The path isn't selected if someone were to select -powerpc-unknown-xcoff as a target for example.  It looks like the Triple.isOSAIX() is true when we we have aix in the target triple. 


================
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
----------------
Xiangling_L wrote:
> Based on the code added in `BackendUtil:551`, should we also add a case for compiling a source to assembly?
Added in lines 15-18


================
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
----------------
Xiangling_L wrote:
> -mabi=vec-extabi is the FE option, should we s/-mabi=vec-extabi/-vec-extabi?
Good catch, fixed. 


================
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));
----------------
Xiangling_L wrote:
> Can we add a testcase for this backend option?
Added in llvm/test/CodeGen/PowerPC/aix-vec-abi.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89684



More information about the cfe-commits mailing list