[cfe-commits] r171755 - in /cfe/trunk: lib/AST/ExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/Sema/SemaExpr.cpp test/CodeGen/catch-undef-behavior.c test/CodeGenOpenCL/shifts.cl test/Sema/shiftOpenCL.cl
Richard Smith
richard at metafoo.co.uk
Mon Jan 7 14:31:06 PST 2013
On Mon, Jan 7, 2013 at 8:43 AM, David Tweed <david.tweed at arm.com> wrote:
> Author: davidtweed
> Date: Mon Jan 7 10:43:27 2013
> New Revision: 171755
>
> URL: http://llvm.org/viewvc/llvm-project?rev=171755&view=rev
> Log:
> Scalar shifts in the OpenCL specification (as of v. 1.2) are defined to be
> with respect to the lower "left-hand-side bitwidth" bits, even when negative);
> see OpenCL spec 6.3j. This patch both implements this behaviour in the code
> generator and "constant folding" bits of Sema, and also prevents tests
> to detect undefinedness in terms of the weaker C99 or C++ specifications
> from being applied.
>
> Added:
> cfe/trunk/test/CodeGenOpenCL/shifts.cl
> cfe/trunk/test/Sema/shiftOpenCL.cl
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/CodeGen/CGExprScalar.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/test/CodeGen/catch-undef-behavior.c
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=171755&r1=171754&r2=171755&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Jan 7 10:43:27 2013
> @@ -4708,9 +4708,14 @@
> return Success(E->getOpcode() == BO_Rem ? LHS % RHS : LHS / RHS, E,
> Result);
> case BO_Shl: {
> - // During constant-folding, a negative shift is an opposite shift. Such
> - // a shift is not a constant expression.
> - if (RHS.isSigned() && RHS.isNegative()) {
> + if (Info.getLangOpts().OpenCL)
> + // OpenCL 6.3j: shift values are effectively % word size of LHS.
> + RHS &= APSInt(llvm::APInt(LHS.getBitWidth(),
> + static_cast<uint64_t>(LHS.getBitWidth() - 1)),
> + RHS.isUnsigned());
> + else if (RHS.isSigned() && RHS.isNegative()) {
> + // During constant-folding, a negative shift is an opposite shift. Such
> + // a shift is not a constant expression.
> CCEDiag(E, diag::note_constexpr_negative_shift) << RHS;
> RHS = -RHS;
> goto shift_right;
> @@ -4735,9 +4740,14 @@
> return Success(LHS << SA, E, Result);
> }
> case BO_Shr: {
> - // During constant-folding, a negative shift is an opposite shift. Such a
> - // shift is not a constant expression.
> - if (RHS.isSigned() && RHS.isNegative()) {
> + if (Info.getLangOpts().OpenCL)
> + // OpenCL 6.3j: shift values are effectively % word size of LHS.
> + RHS &= APSInt(llvm::APInt(LHS.getBitWidth(),
> + static_cast<uint64_t>(LHS.getBitWidth() - 1)),
> + RHS.isUnsigned());
> + else if (RHS.isSigned() && RHS.isNegative()) {
> + // During constant-folding, a negative shift is an opposite shift. Such a
> + // shift is not a constant expression.
> CCEDiag(E, diag::note_constexpr_negative_shift) << RHS;
> RHS = -RHS;
> goto shift_left;
>
> Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=171755&r1=171754&r2=171755&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Mon Jan 7 10:43:27 2013
> @@ -429,6 +429,8 @@
> // Check for undefined division and modulus behaviors.
> void EmitUndefinedBehaviorIntegerDivAndRemCheck(const BinOpInfo &Ops,
> llvm::Value *Zero,bool isDiv);
> + // Common helper for getting how wide LHS of shift is.
> + static Value *GetWidthMinusOneValue(Value* LHS,Value* RHS);
> Value *EmitDiv(const BinOpInfo &Ops);
> Value *EmitRem(const BinOpInfo &Ops);
> Value *EmitAdd(const BinOpInfo &Ops);
> @@ -2365,6 +2367,11 @@
> return Builder.CreateExactSDiv(diffInChars, divisor, "sub.ptr.div");
> }
>
> +Value *ScalarExprEmitter::GetWidthMinusOneValue(Value* LHS,Value* RHS) {
> + unsigned Width = cast<llvm::IntegerType>(LHS->getType())->getBitWidth();
> + return llvm::ConstantInt::get(RHS->getType(), Width - 1);
> +}
> +
> Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
> // LLVM requires the LHS and RHS to be the same type: promote or truncate the
> // RHS to the same size as the LHS.
> @@ -2372,11 +2379,9 @@
> if (Ops.LHS->getType() != RHS->getType())
> RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false, "sh_prom");
>
> - if (CGF.getLangOpts().SanitizeShift &&
> - isa<llvm::IntegerType>(Ops.LHS->getType())) {
> - unsigned Width = cast<llvm::IntegerType>(Ops.LHS->getType())->getBitWidth();
> - llvm::Value *WidthMinusOne =
> - llvm::ConstantInt::get(RHS->getType(), Width - 1);
> + if (CGF.getLangOpts().SanitizeShift && !CGF.getLangOpts().OpenCL
> + && isa<llvm::IntegerType>(Ops.LHS->getType())) {
We usually break lines after the &&, not before.
> + llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, RHS);
> // FIXME: Emit the branching explicitly rather than emitting the check
> // twice.
> EmitBinOpCheck(Builder.CreateICmpULE(RHS, WidthMinusOne), Ops);
> @@ -2401,6 +2406,9 @@
> EmitBinOpCheck(Builder.CreateICmpEQ(BitsShiftedOff, Zero), Ops);
> }
> }
> + // OpenCL 6.3j: shift values are effectively % word size of LHS.
> + if (CGF.getLangOpts().OpenCL)
> + RHS = Builder.CreateAnd(RHS, GetWidthMinusOneValue(Ops.LHS, RHS), "shl.mask");
>
> return Builder.CreateShl(Ops.LHS, RHS, "shl");
> }
> @@ -2412,12 +2420,13 @@
> if (Ops.LHS->getType() != RHS->getType())
> RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false, "sh_prom");
>
> - if (CGF.getLangOpts().SanitizeShift &&
> - isa<llvm::IntegerType>(Ops.LHS->getType())) {
> - unsigned Width = cast<llvm::IntegerType>(Ops.LHS->getType())->getBitWidth();
> - llvm::Value *WidthVal = llvm::ConstantInt::get(RHS->getType(), Width);
> - EmitBinOpCheck(Builder.CreateICmpULT(RHS, WidthVal), Ops);
> - }
> + if (CGF.getLangOpts().SanitizeShift && !CGF.getLangOpts().OpenCL
> + && isa<llvm::IntegerType>(Ops.LHS->getType()))
> + EmitBinOpCheck(Builder.CreateICmpULE(RHS, GetWidthMinusOneValue(Ops.LHS, RHS)), Ops);
> +
> + // OpenCL 6.3j: shift values are effectively % word size of LHS.
> + if (CGF.getLangOpts().OpenCL)
> + RHS = Builder.CreateAnd(RHS, GetWidthMinusOneValue(Ops.LHS, RHS), "shr.mask");
>
> if (Ops.Ty->hasUnsignedIntegerRepresentation())
> return Builder.CreateLShr(Ops.LHS, RHS, "shr");
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=171755&r1=171754&r2=171755&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jan 7 10:43:27 2013
> @@ -6578,6 +6578,11 @@
> static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
> SourceLocation Loc, unsigned Opc,
> QualType LHSType) {
> + // OpenCL 6.3j: shift values are effectively % word size of LHS (more defined),
> + // so skip remaining warnings as we don't want to modify values within Sema.
> + if (S.getLangOpts().OpenCL)
> + return;
> +
> llvm::APSInt Right;
> // Check right/shifter operand
> if (RHS.get()->isValueDependent() ||
>
> Modified: cfe/trunk/test/CodeGen/catch-undef-behavior.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/catch-undef-behavior.c?rev=171755&r1=171754&r2=171755&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/catch-undef-behavior.c (original)
> +++ cfe/trunk/test/CodeGen/catch-undef-behavior.c Mon Jan 7 10:43:27 2013
> @@ -99,7 +99,7 @@
>
> // CHECK: @rsh_inbounds
> int rsh_inbounds(int a, int b) {
> - // CHECK: %[[INBOUNDS:.*]] = icmp ult i32 %[[RHS:.*]], 32
> + // CHECK: %[[INBOUNDS:.*]] = icmp ule i32 %[[RHS:.*]], 31
> // CHECK: br i1 %[[INBOUNDS]]
>
> // CHECK: %[[ARG1:.*]] = zext
>
> Added: cfe/trunk/test/CodeGenOpenCL/shifts.cl
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/shifts.cl?rev=171755&view=auto
> ==============================================================================
> --- cfe/trunk/test/CodeGenOpenCL/shifts.cl (added)
> +++ cfe/trunk/test/CodeGenOpenCL/shifts.cl Mon Jan 7 10:43:27 2013
> @@ -0,0 +1,28 @@
> +// 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.
> +
> +
> +//CHECK: @positiveShift32
> +int positiveShift32(int a,int b) {
> + //CHECK: %shl.mask = and i32 %b, 31
> + //CHECK-NEXT: %shl = shl i32 %a, %shl.mask
> + int c = a<<b;
> + int d = ((int)1)<<33;
> + //CHECK-NEXT: %add = add nsw i32 %shl, 2
> + int e = c + d;
> + //CHECK-NEXT: ret i32 %add
> + return e;
> +}
> +
> +//CHECK: @positiveShift64
> +long positiveShift64(long a,long b) {
> + //CHECK: %shr.mask = and i64 %b, 63
> + //CHECK-NEXT: %shr = ashr i64 %a, %shr.mask
> + long c = a>>b;
> + long d = ((long)8)>>65;
> + //CHECK-NEXT: %add = add nsw i64 %shr, 4
> + long e = c + d;
> + //CHECK-NEXT: ret i64 %add
> + return e;
> +}
>
> Added: cfe/trunk/test/Sema/shiftOpenCL.cl
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/shiftOpenCL.cl?rev=171755&view=auto
> ==============================================================================
> --- cfe/trunk/test/Sema/shiftOpenCL.cl (added)
> +++ cfe/trunk/test/Sema/shiftOpenCL.cl Mon Jan 7 10:43:27 2013
> @@ -0,0 +1,14 @@
> +// 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.
> +
> +//CHECK: @array0 = common global [256 x i8]
> +char array0[((int)1)<<40];
> +//CHECK: @array1 = common global [256 x i8]
> +char array1[((int)1)<<(-24)];
> +
> +//CHECK: @negativeShift32
> +int negativeShift32(int a,int b) {
> + //CHECK: ret i32 65536
> + return ((int)1)<<(-16);
> +}
Sema* tests should generally not depend on CodeGen. How about
replacing the CHECKs with:
char check0[sizeof(array0) == 256 ? 1 : -1];
char check1[sizeof(array1) == 256 ? 1 : -1];
char check2[((int)1 << (-16)) == 65536 ? 1 : -1];
More information about the cfe-commits
mailing list