r228382 - OpenCL: handle shift operator with vector operands
Tom Stellard
tom at stellard.net
Fri Feb 6 09:32:29 PST 2015
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]
> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list