[PATCH] D21678: Fix For pr28288 - Error message in shift of vector values

[Vladimir Yakovlev via cfe-commits]

vbyakovl added inline comments.

================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8597
@@ -8596,4 +8596,3 @@
 ///        by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
-                                       ExprResult &LHS, ExprResult &RHS,
-                                       SourceLocation Loc, bool IsCompAssign) {
+static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
+                                 SourceLocation Loc, bool IsCompAssign) {
----------------
aaron.ballman wrote:
> Why does this drop the mention of OpenCL in the function name?
This routine has common applying rather than for OpenCL only.

================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8638
@@ -8638,4 +8637,3 @@
   if (RHSVecTy) {
-    // OpenCL v1.1 s6.3.j says that for vector types, the operators
-    // are applied component-wise. So if RHS is a vector, then ensure
-    // that the number of elements is the same as LHS...
+    // For vector types, the operators are applied component-wise. So if RHS is
+    // a vector, then ensure that the number of elements is the same as LHS...
----------------
aaron.ballman wrote:
> It's good to keep language references in the comments, so I would leave the reference in even though this is being expanded for non-OpenCL vector types (unless the reference is incorrect).
I'll restore the language reference.

================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683
@@ -8680,5 +8675,3 @@
     }
-    return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
-                               /*AllowBothBool*/true,
-                               /*AllowBoolConversions*/false);
   }
----------------
aaron.ballman wrote:
> Why is it correct to remove semantic checking for vector operands?
This routine targets for checking operands of operations like plus, mul ..., but not shifts. The tests vect_shift_1 and vect_shift_2 (I added) are  examples which were compiled with error.


================
Comment at: llvm/tools/clang/test/Sema/shift.c:75
@@ +74,3 @@
+void
+vect_shift_1 (vec16 *x)
+{
----------------
aaron.ballman wrote:
> Please clang-format the test case.
Ok.



Repository:
  rL LLVM

https://reviews.llvm.org/D21678