r228382 - OpenCL: handle shift operator with vector operands
Jeroen Ketema
j.ketema at imperial.ac.uk
Tue Feb 10 03:55:51 PST 2015
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> 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> 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 <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 <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 <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 <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 <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 <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/803d6964/attachment.html>
More information about the cfe-commits
mailing list