r228382 - OpenCL: handle shift operator with vector operands
Sahasrabuddhe, Sameer
Sameer.Sahasrabuddhe at amd.com
Tue Feb 10 07:54:53 PST 2015
Hello Jeroen,
Thanks for the feedback. Now waiting for an LGTM from Pekka.
Sameer.
________________________________
From: Jeroen Ketema [j.ketema at imperial.ac.uk]
Sent: Tuesday, February 10, 2015 5:25 PM
To: Sahasrabuddhe, Sameer
Cc: llvm cfe; Tom Stellard; Pekka Jääskeläinen
Subject: Re: r228382 - OpenCL: handle shift operator with vector operands
Hi,
I’m not knowledgeable enough to review the updated patch, but all my test cases now pass again.
Jeroen
On 09 Feb 2015, at 11:00, Sameer Sahasrabuddhe <sameer.sahasrabuddhe at amd.com<mailto:sameer.sahasrabuddhe at amd.com>> wrote:
Hi all,
Attached an updated patch that fixes the earlier patch for vector shift operators in OpenCL. Note the new argument "IsCompAssign" for function checkOpenCLVectorShift(), which was not handled in earlier patch.
Sameer.
On 2/6/2015 11:02 PM, Tom Stellard wrote:
On Fri, Feb 06, 2015 at 04:05:22PM +0000, Sahasrabuddhe, Sameer wrote:
Hello Jeroen,
Thanks for pointing this out! I will fix this as soon as possible, but in the meantime, request you or anyone with commit access to please revert my commit! I do not have access to my work computer until Monday morning India time. (Reminder to self: *never* commit on a Friday!).
Hi,
I have reverted this. Can you add this test case when you recommit.
Thanks,
Tom
Sameer.
________________________________________
From: Jeroen Ketema [j.ketema at imperial.ac.uk<mailto:j.ketema at imperial.ac.uk>]
Sent: Friday, February 06, 2015 9:15 PM
To: Sahasrabuddhe, Sameer
Cc: llvm cfe
Subject: Re: r228382 - OpenCL: handle shift operator with vector operands
Hi Sameer,
This commit breaks augmented assignment in combination with shifting.
Consider the following:
typedef __attribute__((ext_vector_type(3))) char char3;
void foo() {
char3 v = {1,1,1};
char3 w = {1,2,3};
w <<= v;
}
If I compile with:
clang -x cl file.c
Then an error is produced:
file.c:10:5: error: expression is not assignable
w <<= v;
~ ^
1 error generated.
This while the OpenCL 1.2 spec says that “w <<= v” is short for “w = w << v”,
and which does not produce an error.
Thanks,
Jeroen
On 06 Feb 2015, at 05:44, Sameer Sahasrabuddhe <sameer.sahasrabuddhe at amd.com<http://amd.com>> wrote:
Author: sameerds
Date: Thu Feb 5 23:44:55 2015
New Revision: 228382
URL: http://llvm.org/viewvc/llvm-project?rev=228382&view=rev
Log:
OpenCL: handle 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.
Relax the requirement for "usual arithmetic conversions":
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; }
Consolidate existing tests under CodeGenOpenCL, and add more tests
under SemaOpenCL.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/CodeGenOpenCL/shifts.cl
cfe/trunk/test/SemaOpenCL/shifts.cl
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=228382&r1=228381&r2=228382&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Feb 5 23:44:55 2015
@@ -2031,6 +2031,8 @@ def err_typecheck_vector_not_convertable
"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 :
"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">,
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=228382&r1=228381&r2=228382&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Feb 5 23:44:55 2015
@@ -7772,6 +7772,67 @@ static void DiagnoseBadShiftValues(Sema&
<< 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(ExprRe
// 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
Modified: cfe/trunk/test/CodeGenOpenCL/shifts.cl
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/shifts.cl?rev=228382&r1=228381&r2=228382&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenOpenCL/shifts.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/shifts.cl Thu Feb 5 23:44:55 2015
@@ -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.
+
+// 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)];
-//CHECK: @positiveShift32
+ // 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;
}
Modified: cfe/trunk/test/SemaOpenCL/shifts.cl
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/shifts.cl?rev=228382&r1=228381&r2=228382&view=diff
==============================================================================
--- cfe/trunk/test/SemaOpenCL/shifts.cl (original)
+++ cfe/trunk/test/SemaOpenCL/shifts.cl Thu Feb 5 23:44:55 2015
@@ -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)];
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
- // CHECK: ret i32 65536
- return ((int)1)<<(-16);
+// 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}}
}
_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
<vector-shift-fixed.patch>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150210/ab4b3c96/attachment.html>
More information about the cfe-commits
mailing list