Fixed in r171820. ...Takumi
2013/1/8 Dmitri Gribenko <[email protected]>: > On Mon, Jan 7, 2013 at 6:43 PM, David Tweed <[email protected]> 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())) { >> + 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; >> +} > > This test breaks on a Release-Asserts build with CMake+ninja. The > generated IR is: > > define i32 @positiveShift32(i32 %a, i32 %b) nounwind readnone { > %1 = and i32 %b, 31 > %2 = shl i32 %a, %1 > %3 = add nsw i32 %2, 2 > ret i32 %3 > } > > define i64 @positiveShift64(i64 %a, i64 %b) nounwind readnone { > %1 = and i64 %b, 63 > %2 = ashr i64 %a, %1 > %3 = add nsw i64 %2, 4 > ret i64 %3 > } > > Dmitri > > -- > main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if > (j){printf("%d\n",i);}}} /*Dmitri Gribenko <[email protected]>*/ > _______________________________________________ > 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
