[PATCH] D11001: Add support for System z vector language extensions

Richard Smith richard at metafoo.co.uk
Tue Jul 7 14:01:07 PDT 2015


Is this extension documented anywhere?


================
Comment at: include/clang/Basic/LangOptions.def:107
@@ -106,2 +106,3 @@
 LANGOPT(AltiVec           , 1, 0, "AltiVec-style vector initializers")
+LANGOPT(ZVector           , 1, 0, "System z vector extensions")
 LANGOPT(Exceptions        , 1, 0, "exception handling")
----------------
Should this "z" be uppercase (here and elsewhere in the patch)? Currently in Clang's comments we always spell this "SystemZ" (with no space); we should ideally only have a single typographical convention for this.

================
Comment at: include/clang/Driver/Options.td:1330-1333
@@ -1329,1 +1329,6 @@
 
+def mzvector : Flag<["-"], "mzvector">, Group<m_Group>, Flags<[CC1Option]>,
+  HelpText<"Enable System z vector language extension">;
+def mno_zvector : Flag<["-"], "mno-zvector">, Group<m_Group>,
+  Flags<[CC1Option]>;
+
----------------
This should be a `-f` flag, not a `-m` flag. (I think we only support `-maltivec` for GCC compatibility.)

================
Comment at: lib/CodeGen/CGExprScalar.cpp:2859-2860
@@ -2858,3 +2858,4 @@
     // intrinsics comparing vectors and giving 0 or 1 as a result
-    if (LHSTy->isVectorType() && !E->getType()->isVectorType()) {
+    if (!CGF.getLangOpts().ZVector &&
+        LHSTy->isVectorType() && !E->getType()->isVectorType()) {
       // constants for mapping CR6 register bits to predicate result
----------------
The code after this `if` will not do the right thing in this case. Why do you need this change? What are the semantics of a vector comparison that produces a scalar under this extension?

================
Comment at: lib/Driver/Tools.cpp:3955-3959
@@ -3952,1 +3954,7 @@
 
+  // Similarly for -mzvector and SystemZ.
+  if (const Arg *A = Args.getLastArg(options::OPT_mzvector))
+    if (!(getToolChain().getArch() == llvm::Triple::systemz))
+      D.Diag(diag::err_drv_argument_only_allowed_with)
+        << A->getAsString(Args) << "systemz";
+
----------------
What is the reason for this restriction? Would the extension not work in some way on other architectures? For `-faltivec`, we use `ppc_altivec_vcmp*` intrinsics for some comparisons (and even that is tenuous justification, because these intrinsics could be represented directly in IR), but there doesn't seem to be anything comparable in this case?


http://reviews.llvm.org/D11001







More information about the cfe-commits mailing list