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!).
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
