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