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.
https://gcc.gnu.org/onlinedocs/gcc-6.2.0/gcc/Vector-Extensions.html#Vector-Extensions
https://reviews.llvm.org/D25866
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
