vbyakovlcl added inline comments.
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8733
@@ -8731,2 +8732,3 @@
if (!IsCompAssign) {
- LHS = S.UsualUnaryConversions(LHS.get());
+ if (S.LangOpts.OpenCL || S.LangOpts.ZVector)
+ LHS = S.UsualUnaryConversions(LHS.get());
----------------
ahatanak wrote:
> It wasn't clear to me why we have to call
> DefaultFunctionArrayLvalueConversion instead of UsualUnaryConversions. Is it
> possible to come up with a test case that would fail without this change, and
> if it is, can you add it to vecshift.c?
I cannot remember why I did this change. Removing it doesn't cause any new
fails.
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8747
@@ -8742,3 +8746,3 @@
// Note that RHS might not be a vector.
QualType RHSType = RHS.get()->getType();
----------------
ahatanak wrote:
> We no longer error out when LHS is not a vector, so it should mention that
> either the LHS or the RHS might not be a vector.
I added a comment.
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
@@ -8770,1 +8786,3 @@
}
+ if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+ const BuiltinType *LHSBT = LHSEleType->getAs<clang::BuiltinType>();
----------------
ahatanak wrote:
> Is it possible to split this out to another patch? The first patch would fix
> handling of (scalar << vector) and the second patch would make clang reject
> vector shifts if element sizes of the LHS and RHS are different. It's not
> clear whether it's correct to reject the latter case, so perhaps you can
> discuss it in a separate patch?
Ok. I removed this and will fail a new review after committing of this review.
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8847
@@ -8774,3 +8846,3 @@
S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
}
----------------
ahatanak wrote:
> When the LHS is a scalar, we check the type of the LHS and the element type
> of the RHS, and if necessary, cast the LHS to the element type of the RHS.
> What's the reason we don't do the same here?
No need, because CodeGen inserts a conversion.
define <8 x i16> @foo1(<8 x i16> %n, i32 %m) #0 {
entry:
%n.addr = alloca <8 x i16>, align 16
%m.addr = alloca i32, align 4
store <8 x i16> %n, <8 x i16>* %n.addr, align 16
store i32 %m, i32* %m.addr, align 4
%0 = load <8 x i16>, <8 x i16>* %n.addr, align 16
%1 = load i32, i32* %m.addr, align 4
%splat.splatinsert = insertelement <8 x i32> undef, i32 %1, i32 0
%splat.splat = shufflevector <8 x i32> %splat.splatinsert, <8 x i32> undef,
<8 x i32> zeroinitializer
%sh_prom = trunc <8 x i32> %splat.splat to <8 x i16>
%shl = shl <8 x i16> %0, %sh_prom
ret <8 x i16> %shl
We need insert a conversion of scalar LHS to the RHS element type because we
must return a vector of the RHS type.
https://reviews.llvm.org/D24467
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
