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

[Ulrich Weigand]

uweigand marked 2 inline comments as done.
uweigand added a comment.

In http://reviews.llvm.org/D11001#201446, @rsmith wrote:

> 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].


Thanks!

> 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...)


AltiVec has allowed "vector long", but this has always been causing issues when writing code supporting both 32-bit and 64-bit mode, since the base "long" type changes size.  Should "vector long" be four 32-bit or two 64-bit values, or should that change depending on compilation mode?  Either choice causes surprises to the user in some circumstances ...

Because of that, "vector long" has been considered "deprecated" for AltiVec for at least 10 years now, and the recommendation is to use either "vector int" or else "vector long long".  For the new System z extension, we therefore started out with "vector long" not just deprecated, but unsupported.  (For clang, this is not so much of an issue since we currently support only 64-bit mode on System z, but both GCC and XLC do support 32-bit mode on System z as well, so there's the same issue there.)


================
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))
----------------
rsmith wrote:
> 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.)
Good point.  I've added code to reject the combination.

================
Comment at: lib/Sema/SemaExpr.cpp:7430
@@ +7429,3 @@
+    return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
+                               /*AllowBothBool*/!getLangOpts().ZVector,
+                               /*AllowBoolConversions*/false);
----------------
rsmith wrote:
> 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.
OK, updated accordingly.


http://reviews.llvm.org/D11001