r228382 - OpenCL: handle shift operator with vector operands
Sahasrabuddhe, Sameer
Sameer.Sahasrabuddhe at amd.com
Fri Feb 6 08:05:22 PST 2015
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 [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}}
> }
>
More information about the cfe-commits
mailing list