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 <[email protected]> > 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 [[email protected] >>> <mailto:[email protected]>] >>> 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 >>> [email protected] <mailto:[email protected]> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits> > > > <vector-shift-fixed.patch>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
