[PATCH][OPENCL] shift operator with vector operands

Sameer Sahasrabuddhe sameer.sahasrabuddhe at amd.com
Thu Feb 5 02:43:49 PST 2015


This patch introduces a number of checks when the shift operator is used 
on OpenCL vectors:

 1. If LHS is a scalar, then RHS cannot be a vector.
 2. Operands must be of integer type.
 3. If both are vectors, then the number of elements must match.

Also, when LHS is a vector, a scalar RHS can simply be expanded into a 
vector; OpenCL does not require that its rank be lower than the LHS for 
"usual arithmetic conversions" defined in Section 6.2.6. This is because 
unlike other binary operators, the shift operators do not actually 
combine their operands. This is further supported by the fact that in 
OpenCL, the shift (RHS) is computed modulo the word-size of the LHS, and 
the effective shift value can always fit in a smaller type. For example, 
the following code is not an error even if the implicit type of the 
constant literal is "int":

     char2 foo(char2 v) { return v << 1; }

The patch additionally consolidates existing tests under CodeGenOpenCL, 
and creates more tests under SemaOpenCL.

Sameer.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150205/cef4695d/attachment.html>
-------------- next part --------------
commit 56cdf6444a98a58f7cc6e296c7d814c95dab4161
Author: Sameer Sahasrabuddhe <sameer.sahasrabuddhe at amd.com>
Date:   Thu Feb 5 15:30:04 2015 +0530

    OpenCL: shift operator with vector operands
    
    Introduce a number of checks:
    1. If LHS is a scalar, then RHS cannot be a vector.
    2. Operands must be of integer type.
    3. If both are vectors, then the number of elements must match.
    
    Also, when LHS is a vector, a scalar RHS can simply be expanded into a
    vector; OpenCL does not require that its rank be lower than the LHS.
    For example, the following code is not an error even if the implicit
    type of the constant literal is "int".
    
      char2 foo(char2 v) { return v << 1; }
    
    Consolidated existing tests under CodeGenOpenCL, and created more
    tests under SemaOpenCL.

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 4f68d10..65ee955 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2031,6 +2031,8 @@ def err_typecheck_vector_not_convertable : Error<
   "can't convert between vector values of different size (%0 and %1)">;
 def err_typecheck_vector_not_convertable_non_scalar : Error<
   "can't convert between vector and non-scalar values (%0 and %1)">;
+def err_typecheck_vector_lengths_not_equal : Error<
+  "vector operands do not have the same number of elements (%0 and %1)">;
 def err_ext_vector_component_exceeds_length : Error<
   "vector component access exceeds type %0">;
 def err_ext_vector_component_name_illegal : Error<
@@ -4884,6 +4886,8 @@ def err_ivar_reference_type : Error<
   "instance variables cannot be of reference type">;
 def err_typecheck_illegal_increment_decrement : Error<
   "cannot %select{decrement|increment}1 value of type %0">;
+def err_typecheck_expect_int : Error<
+  "used type %0 where integer is required">;
 def err_typecheck_arithmetic_incomplete_type : Error<
   "arithmetic on a pointer to an incomplete type %0">;
 def err_typecheck_pointer_arith_function_type : Error<
@@ -5047,6 +5051,9 @@ def warn_null_in_comparison_operation : Warning<
   "comparison between NULL and non-pointer "
   "%select{(%1 and NULL)|(NULL and %1)}0">,
   InGroup<NullArithmetic>;
+def err_shift_rhs_only_vector : Error<
+  "requested shift is a vector of type %0 but the first operand is not a "
+  "vector (%1)">;
 
 def warn_logical_not_on_lhs_of_comparison : Warning<
   "logical not is only applied to the left hand side of this comparison">,
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 2b4c639..099238d 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -7772,6 +7772,67 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
     << RHS.get()->getSourceRange();
 }
 
+/// \brief Return the resulting type when an OpenCL vector is shifted
+///        by a scalar or vector shift amount.
+static QualType checkOpenCLVectorShift(Sema &S,
+                                       ExprResult &LHS, ExprResult &RHS,
+                                       SourceLocation Loc) {
+  // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector.
+  if (!LHS.get()->getType()->isVectorType()) {
+    S.Diag(Loc, diag::err_shift_rhs_only_vector)
+      << RHS.get()->getType() << LHS.get()->getType()
+      << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+    return QualType();
+  }
+
+  LHS = S.UsualUnaryConversions(LHS.get());
+  if (LHS.isInvalid()) return QualType();
+
+  RHS = S.UsualUnaryConversions(RHS.get());
+  if (RHS.isInvalid()) return QualType();
+
+  QualType LHSType = LHS.get()->getType();
+  const VectorType *LHSVecTy = LHSType->getAs<VectorType>();
+  QualType LHSEleType = LHSVecTy->getElementType();
+
+  // Note that RHS might not be a vector.
+  QualType RHSType = RHS.get()->getType();
+  const VectorType *RHSVecTy = RHSType->getAs<VectorType>();
+  QualType RHSEleType = RHSVecTy ? RHSVecTy->getElementType() : RHSType;
+
+  // OpenCL v1.1 s6.3.j says that the operands need to be integers.
+  if (!LHSEleType->isIntegerType()) {
+    S.Diag(Loc, diag::err_typecheck_expect_int)
+      << LHS.get()->getType() << LHS.get()->getSourceRange();
+    return QualType();
+  }
+
+  if (!RHSEleType->isIntegerType()) {
+    S.Diag(Loc, diag::err_typecheck_expect_int)
+      << RHS.get()->getType() << RHS.get()->getSourceRange();
+    return QualType();
+  }
+
+  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...
+    if (RHSVecTy->getNumElements() != LHSVecTy->getNumElements()) {
+      S.Diag(Loc, diag::err_typecheck_vector_lengths_not_equal)
+        << LHS.get()->getType() << RHS.get()->getType()
+        << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+      return QualType();
+    }
+  } else {
+    // ...else expand RHS to match the number of elements in LHS.
+    QualType VecTy =
+      S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
+    RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
+  }
+
+  return LHSType;
+}
+
 // C99 6.5.7
 QualType Sema::CheckShiftOperands(ExprResult &LHS, ExprResult &RHS,
                                   SourceLocation Loc, unsigned Opc,
@@ -7780,8 +7841,11 @@ QualType Sema::CheckShiftOperands(ExprResult &LHS, ExprResult &RHS,
 
   // Vector shifts promote their scalar inputs to vector type.
   if (LHS.get()->getType()->isVectorType() ||
-      RHS.get()->getType()->isVectorType())
+      RHS.get()->getType()->isVectorType()) {
+    if (LangOpts.OpenCL)
+      return checkOpenCLVectorShift(*this, LHS, RHS, Loc);
     return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign);
+  }
 
   // Shifts don't perform usual arithmetic conversions, they just do integer
   // promotions on each operand. C99 6.5.7p3
diff --git a/test/CodeGenOpenCL/shifts.cl b/test/CodeGenOpenCL/shifts.cl
index 015a777..ab64051 100644
--- a/test/CodeGenOpenCL/shifts.cl
+++ b/test/CodeGenOpenCL/shifts.cl
@@ -1,57 +1,73 @@
-// RUN: %clang_cc1 -x cl -O1 -emit-llvm  %s -o - -triple x86_64-linux-gnu | FileCheck %s
-// OpenCL essentially reduces all shift amounts to the last word-size bits before evaluating.
-// Test this both for variables and constants evaluated in the front-end.
+// RUN: %clang_cc1 -x cl -O1 -emit-llvm  %s -o - -triple x86_64-linux-gnu | FileCheck %s -check-prefix=OPT
+// RUN: %clang_cc1 -x cl -O0 -emit-llvm  %s -o - -triple x86_64-linux-gnu | FileCheck %s -check-prefix=NOOPT
 
+// OpenCL essentially reduces all shift amounts to the last word-size
+// bits before evaluating. Test this both for variables and constants
+// evaluated in the front-end.
 
-//CHECK: @positiveShift32
+// OPT: @gtest1 = constant i64 2147483648
+__constant const unsigned long gtest1 = 1UL << 31;
+
+// NOOPT: @negativeShift32
+int negativeShift32(int a,int b) {
+  // NOOPT: %array0 = alloca [256 x i8]
+  char array0[((int)1)<<40];
+  // NOOPT: %array1 = alloca [256 x i8]
+  char array1[((int)1)<<(-24)];
+
+  // NOOPT: ret i32 65536
+  return ((int)1)<<(-16);
+}
+
+//OPT: @positiveShift32
 int positiveShift32(int a,int b) {
-  //CHECK: [[M32:%.+]] = and i32 %b, 31
-  //CHECK-NEXT: [[C32:%.+]] = shl i32 %a, [[M32]]
+  //OPT: [[M32:%.+]] = and i32 %b, 31
+  //OPT-NEXT: [[C32:%.+]] = shl i32 %a, [[M32]]
   int c = a<<b;
   int d = ((int)1)<<33;
-  //CHECK-NEXT: [[E32:%.+]] = add nsw i32 [[C32]], 2
+  //OPT-NEXT: [[E32:%.+]] = add nsw i32 [[C32]], 2
   int e = c + d;
-  //CHECK-NEXT: ret i32 [[E32]]
+  //OPT-NEXT: ret i32 [[E32]]
   return e;
 }
 
-//CHECK: @positiveShift64
+//OPT: @positiveShift64
 long positiveShift64(long a,long b) {
-  //CHECK: [[M64:%.+]] = and i64 %b, 63
-  //CHECK-NEXT: [[C64:%.+]] = ashr i64 %a, [[M64]]
+  //OPT: [[M64:%.+]] = and i64 %b, 63
+  //OPT-NEXT: [[C64:%.+]] = ashr i64 %a, [[M64]]
   long c = a>>b;
   long d = ((long)8)>>65;
-  //CHECK-NEXT: [[E64:%.+]] = add nsw i64 [[C64]], 4
+  //OPT-NEXT: [[E64:%.+]] = add nsw i64 [[C64]], 4
   long e = c + d;
-  //CHECK-NEXT: ret i64 [[E64]]
+  //OPT-NEXT: ret i64 [[E64]]
   return e;
 }
 
 typedef __attribute__((ext_vector_type(4))) int int4;
 
-//CHECK: @vectorVectorTest
+//OPT: @vectorVectorTest
 int4 vectorVectorTest(int4 a,int4 b) {
-  //CHECK: [[VM:%.+]] = and <4 x i32> %b, <i32 31, i32 31, i32 31, i32 31>
-  //CHECK-NEXT: [[VC:%.+]] = shl <4 x i32> %a, [[VM]]
+  //OPT: [[VM:%.+]] = and <4 x i32> %b, <i32 31, i32 31, i32 31, i32 31>
+  //OPT-NEXT: [[VC:%.+]] = shl <4 x i32> %a, [[VM]]
   int4 c = a << b;
-  //CHECK-NEXT: [[VF:%.+]] = add <4 x i32> [[VC]], <i32 2, i32 4, i32 16, i32 8>
+  //OPT-NEXT: [[VF:%.+]] = add <4 x i32> [[VC]], <i32 2, i32 4, i32 16, i32 8>
   int4 d = {1, 1, 1, 1};
   int4 e = {33, 34, -28, -29};
   int4 f = c + (d << e);
-  //CHECK-NEXT: ret <4 x i32> [[VF]]
+  //OPT-NEXT: ret <4 x i32> [[VF]]
   return f;
 }
 
-//CHECK: @vectorScalarTest
+//OPT: @vectorScalarTest
 int4 vectorScalarTest(int4 a,int b) {
-  //CHECK: [[SP0:%.+]] = insertelement <4 x i32> undef, i32 %b, i32 0
-  //CHECK: [[SP1:%.+]] = shufflevector <4 x i32> [[SP0]], <4 x i32> undef, <4 x i32> zeroinitializer
-  //CHECK: [[VSM:%.+]] = and <4 x i32> [[SP1]], <i32 31, i32 31, i32 31, i32 31>
-  //CHECK-NEXT: [[VSC:%.+]] = shl <4 x i32> %a, [[VSM]]
+  //OPT: [[SP0:%.+]] = insertelement <4 x i32> undef, i32 %b, i32 0
+  //OPT: [[SP1:%.+]] = shufflevector <4 x i32> [[SP0]], <4 x i32> undef, <4 x i32> zeroinitializer
+  //OPT: [[VSM:%.+]] = and <4 x i32> [[SP1]], <i32 31, i32 31, i32 31, i32 31>
+  //OPT-NEXT: [[VSC:%.+]] = shl <4 x i32> %a, [[VSM]]
   int4 c = a << b;
-  //CHECK-NEXT: [[VSF:%.+]] = add <4 x i32> [[VSC]], <i32 4, i32 4, i32 4, i32 4>
+  //OPT-NEXT: [[VSF:%.+]] = add <4 x i32> [[VSC]], <i32 4, i32 4, i32 4, i32 4>
   int4 d = {1, 1, 1, 1};
   int4 f = c + (d << 34);
-  //CHECK-NEXT: ret <4 x i32> [[VSF]]
+  //OPT-NEXT: ret <4 x i32> [[VSF]]
   return f;
 }
diff --git a/test/SemaOpenCL/shifts.cl b/test/SemaOpenCL/shifts.cl
index 5b0c6fb..dba1400 100644
--- a/test/SemaOpenCL/shifts.cl
+++ b/test/SemaOpenCL/shifts.cl
@@ -1,17 +1,54 @@
-// RUN: %clang_cc1 -x cl -O0 -emit-llvm  %s -o - -triple x86_64-linux-gnu | FileCheck %s
-// OpenCL essentially reduces all shift amounts to the last word-size bits before evaluating.
-// Test this both for variables and constants evaluated in the front-end.
-
-// CHECK: @gtest1 = constant i64 2147483648
-__constant const unsigned long gtest1 = 1UL << 31;
-
-// CHECK: @negativeShift32
-int negativeShift32(int a,int b) {
-  // CHECK: %array0 = alloca [256 x i8]
-  char array0[((int)1)<<40];
-  // CHECK: %array1 = alloca [256 x i8]
-  char array1[((int)1)<<(-24)];
-
-  // CHECK: ret i32 65536
-  return ((int)1)<<(-16);
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+
+typedef __attribute__((ext_vector_type(2))) char char2;
+typedef __attribute__((ext_vector_type(3))) char char3;
+
+typedef __attribute__((ext_vector_type(2))) int int2;
+
+typedef __attribute__((ext_vector_type(2))) float float2;
+
+// ** Positive tests **
+
+char2 ptest01(char2 c, char s) {
+  return c << s;
+}
+
+char2 ptest02(char2 c, char2 s) {
+  return c << s;
+}
+
+char2 ptest03(char2 c, int s) {
+  return c << s;
+}
+
+char2 ptest04(char2 c, int2 s) {
+  return c << s;
+}
+
+int2 ptest05(int2 c, char2 s) {
+  return c << s;
+}
+
+char2 ptest06(char2 c) {
+  return c << 1;
+}
+
+// ** Negative tests **
+
+char2 ntest01(char c, char2 s) {
+  return c << s; // expected-error {{requested shift is a vector of type 'char2' (vector of 2 'char' values) but the first operand is not a vector ('char')}}
+}
+
+char3 ntest02(char3 c, char2 s) {
+  return c << s; // expected-error {{vector operands do not have the same number of elements ('char3' (vector of 3 'char' values) and 'char2' (vector of 2 'char' values))}}
+}
+
+float2 ntest03(float2 c, char s) {
+  return c << s; // expected-error {{used type 'float2' (vector of 2 'float' values) where integer is required}}
+}
+
+int2 ntest04(int2 c, float s) {
+  return c << s; // expected-error {{used type 'float' where integer is required}}
 }


More information about the cfe-commits mailing list