[PATCH] D24467: Fix an error after D21678

[Vladimir Yakovlev via cfe-commits]

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