[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D11001: Add support for System z vector language extensions
From:       Richard Smith <richard () metafoo ! co ! uk>
Date:       2015-07-09 0:06:18
Message-ID: 9dd8624f6546291f5873b6e9ad0dfc58 () localhost ! localdomain
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]