[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

Dmitri Gribenko gribozavr at gmail.com
Mon Jan 7 11:12:15 PST 2013


On Mon, Jan 7, 2013 at 6:43 PM, 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())) {
> +    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 <gribozavr at gmail.com>*/



More information about the cfe-commits mailing list