[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

Simon Dardis via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 16 07:47:57 PST 2016

sdardis added inline comments.

Comment at: lib/Sema/SemaExpr.cpp:8051
+  if (!LHSVecType) {
+    assert(RHSVecType && "RHSVecType is not a vector!");
     if (!tryVectorConvertAndSplat(*this, (IsCompAssign ? nullptr : &LHS),
bruno wrote:
> `tryVectorConvertAndSplat` does more than plain scalar splat; it supports a more general type of CK_IntegralCast, see the comment on one of your changes to the tests below.
> I suggest that instead of reusing this function, you should create another one that only handles the cases we actually want to support for non-ext vectors (i.e. for GCC compat). 
(ignore this, phabricator's web interface is acting up)

Comment at: lib/Sema/SemaExpr.cpp:8267
+  }
   // Otherwise, use the generic diagnostic.
bruno wrote:
> sdardis wrote:
> > bruno wrote:
> > > This change seems orthogonal to this patch. Can you make it a separated patch with the changes from test/Sema/vector-cast.c?
> > This change is a necessary part of this patch and it can't be split out in sensible fashion.
> > 
> > For example, line 329 of test/Sema/zvector.c adds a scalar signed character to a vector of signed characters. With just this change we would report "cannot convert between scalar type 'signed char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation".
> > 
> > This is heavily misleading for C and C++ code as we don't perform implicit conversions and signed char could be implicitly converted to a vector of signed char without the loss of precision.
> > 
> > To make this diagnostic precise without performing implicit conversions requires determining cases where implicit conversion would cause truncation and reporting that failure reason, or defaulting to the generic diagnostic.
> > 
> > Keeping this change as part of this patch is cleaner and simpler as it covers both implicit conversions and more precise diagnostics.
> Can you double check whether we should support these GCC semantics for getLangOpts().ZVector? If not, then this should be introduced in a separate patch/commit.
See my comment above.

Comment at: lib/Sema/SemaExpr.cpp:8171
+        return LHSType;
+    }
bruno wrote:
> It's not clear to me whether clang should support the GCC semantics when in `getLangOpts().AltiVec || getLangOpts().ZVector`, do you?
> You can remove the curly braces for the `if` and `else` here.
We should, GCC performs scalar to vector conversions regardless of what cpu features are available. If the target machine doesn't have vector support, GCC silently scalarizes the operation.



More information about the cfe-commits mailing list