rsmith added a comment.

I think it's reasonable for Clang to natively support this extension [especially since GCC and xlC support this, there is presumably a significant amount of existing code that uses this extension on System z, there is a lot of overlap with our existing Altivec extension, and we have a code owner with a strong track record proposing it].

The patch looks to be in really good shape. (FWIW, I find it a bit weird that `vector long` is not valid but all the other vectors of integral types are, but hey, it's your extension...)


================
Comment at: lib/Driver/Tools.cpp:3955-3959
@@ -3952,7 +3954,7 @@
 
   if (getToolChain().SupportsProfiling())
     Args.AddLastArg(CmdArgs, options::OPT_pg);
 
   // -flax-vector-conversions is default.
   if (!Args.hasFlag(options::OPT_flax_vector_conversions,
                     options::OPT_fno_lax_vector_conversions))
----------------
Thanks.

We should probably reject `-faltivec -fzvector`, since they give different semantics for certain vector operations. (Either that, or we need to pick which one wins in each case, which doesn't sound great.)

================
Comment at: lib/Sema/SemaExpr.cpp:7430
@@ +7429,3 @@
+    return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
+                               /*AllowBothBool*/!getLangOpts().ZVector,
+                               /*AllowBoolConversions*/false);
----------------
It'd be better to phrase this as a positive language mode check than a negative one (that is, `AllowbothBool = getLangOpts().Altivec`) -- generally, where possible, we should aim for LangOptions values to enable features rather than disable them.


http://reviews.llvm.org/D11001




_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits