[llvm-commits] [llvm] r169182 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombine.h lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp test/Transforms/InstCombine/2010-11-01-lshr-mask.ll test/Transforms/InstCombine/shift.ll test/Transforms/InstCombine/signext.ll

Bill Wendling wendling at apple.com
Mon Dec 3 16:44:01 PST 2012


Hi Shuxin,

I don't think any of these transformations are correct:

> rdar://12329730 (2nd part)
> 
> This change tries to simmplify E1 = " X >> C1 << C2" into :
>  - E2 = "X << (C2 - C1)" if C2 > C1, or

	X = 7; X >> 1 << 2 == 0xC
	X = 7; X << 1 == 0xE

>  - E2 = "X >> (C1 - C2)" if C1 > C2, or

	X = 7; X >> 2 << 1 == 0x2
	X = 7; X >> 1 == 0x3

>  - E2 = X if C1 == C2.

Is this correct? If we have:

	X >> 1 << 1

I expect it to clear out the 0th bit. But with the above transformation, it won't do that.

-bw

> Reviewed by Nadav. Thanks!
> 
> Modified:
>    llvm/trunk/lib/Transforms/InstCombine/InstCombine.h
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
>    llvm/trunk/test/Transforms/InstCombine/2010-11-01-lshr-mask.ll
>    llvm/trunk/test/Transforms/InstCombine/shift.ll
>    llvm/trunk/test/Transforms/InstCombine/signext.ll
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombine.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombine.h?rev=169182&r1=169181&r2=169182&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombine.h (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombine.h Mon Dec  3 18:04:54 2012
> @@ -327,6 +327,11 @@
>   bool SimplifyDemandedBits(Use &U, APInt DemandedMask, 
>                             APInt& KnownZero, APInt& KnownOne,
>                             unsigned Depth=0);
> +  /// Helper routine of SimplifyDemandedUseBits. It tries to simplify demanded
> +  /// bit for "r1 = shr x, c1; r2 = shl r1, c2" instruction sequence.
> +  Value *SimplifyShrShlDemandedBits(Instruction *Lsr, Instruction *Sftl,
> +                                    APInt DemandedMask, APInt &KnownZero,
> +                                    APInt &KnownOne);
> 
>   /// SimplifyDemandedInstructionBits - Inst is an integer instruction that
>   /// SimplifyDemandedBits knows about.  See if the instruction has any
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp?rev=169182&r1=169181&r2=169182&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp Mon Dec  3 18:04:54 2012
> @@ -16,9 +16,10 @@
> #include "InstCombine.h"
> #include "llvm/DataLayout.h"
> #include "llvm/IntrinsicInst.h"
> +#include "llvm/Support/PatternMatch.h"
> 
> using namespace llvm;
> -
> +using namespace llvm::PatternMatch;
> 
> /// ShrinkDemandedConstant - Check to see if the specified operand of the 
> /// specified instruction is a constant integer.  If so, check to see if there
> @@ -580,6 +581,17 @@
>     break;
>   case Instruction::Shl:
>     if (ConstantInt *SA = dyn_cast<ConstantInt>(I->getOperand(1))) {
> +      {
> +        Value *VarX; ConstantInt *C1;
> +        if (match(I->getOperand(0), m_Shr(m_Value(VarX), m_ConstantInt(C1)))) {
> +          Instruction *Shr = cast<Instruction>(I->getOperand(0));
> +          Value *R = SimplifyShrShlDemandedBits(Shr, I, DemandedMask,
> +                                                KnownZero, KnownOne);
> +          if (R)
> +            return R;
> +        }
> +      }
> +
>       uint64_t ShiftAmt = SA->getLimitedValue(BitWidth-1);
>       APInt DemandedMaskIn(DemandedMask.lshr(ShiftAmt));
> 
> @@ -800,6 +812,78 @@
>   return 0;
> }
> 
> +/// Helper routine of SimplifyDemandedUseBits. It tries to simplify
> +/// "E1 = (X lsr C1) << C2", where the C1 and C2 are constant, into
> +/// "E2 = X << (C2 - C1)" or "E2 = X >> (C1 - C2)", depending on the sign
> +/// of "C2-C1".
> +///
> +/// Suppose E1 and E2 are generally different in bits S={bm, bm+1,
> +/// ..., bn}, without considering the specific value X is holding.
> +/// This transformation is legal iff one of following conditions is hold:
> +///  1) All the bit in S are 0, in this case E1 == E2.
> +///  2) We don't care those bits in S, per the input DemandedMask.
> +///  3) Combination of 1) and 2). Some bits in S are 0, and we don't care the
> +///     rest bits.
> +///
> +/// Currently we only test condition 2).
> +///
> +/// As with SimplifyDemandedUseBits, it returns NULL if the simplification was
> +/// not successful.
> +Value *InstCombiner::SimplifyShrShlDemandedBits(Instruction *Shr,
> +  Instruction *Shl, APInt DemandedMask, APInt &KnownZero, APInt &KnownOne) {
> +
> +  unsigned ShlAmt = cast<ConstantInt>(Shl->getOperand(1))->getZExtValue();
> +  unsigned ShrAmt = cast<ConstantInt>(Shr->getOperand(1))->getZExtValue();
> +
> +  KnownOne.clearAllBits();
> +  KnownZero = APInt::getBitsSet(KnownZero.getBitWidth(), 0, ShlAmt-1);
> +  KnownZero &= DemandedMask;
> +
> +  if (ShlAmt == 0 || ShrAmt == 0)
> +    return 0;
> +
> +  Value *VarX = Shr->getOperand(0);
> +  Type *Ty = VarX->getType();
> +
> +  APInt BitMask1(Ty->getIntegerBitWidth(), (uint64_t)-1);
> +  APInt BitMask2(Ty->getIntegerBitWidth(), (uint64_t)-1);
> +
> +  bool isLshr = (Shr->getOpcode() == Instruction::LShr);
> +  BitMask1 = isLshr ? (BitMask1.lshr(ShrAmt) << ShlAmt) :
> +                      (BitMask1.ashr(ShrAmt) << ShlAmt);
> +
> +  if (ShrAmt <= ShlAmt) {
> +    BitMask2 <<= (ShlAmt - ShrAmt);
> +  } else {
> +    BitMask2 = isLshr ? BitMask2.lshr(ShrAmt - ShlAmt):
> +                        BitMask2.ashr(ShrAmt - ShlAmt);
> +  }
> +
> +  // Check if condition-2 (see the comment to this function) is satified.
> +  if ((BitMask1 & DemandedMask) == (BitMask2 & DemandedMask)) {
> +    if (ShrAmt == ShlAmt)
> +      return VarX;
> +
> +    if (!Shr->hasOneUse())
> +      return 0;
> +
> +    BinaryOperator *New;
> +    if (ShrAmt < ShlAmt) {
> +      Constant *Amt = ConstantInt::get(VarX->getType(), ShlAmt - ShrAmt);
> +      New = BinaryOperator::CreateShl(VarX, Amt);
> +      BinaryOperator *Orig = cast<BinaryOperator>(Shl);
> +      New->setHasNoSignedWrap(Orig->hasNoSignedWrap());
> +      New->setHasNoUnsignedWrap(Orig->hasNoUnsignedWrap());
> +    } else {
> +      Constant *Amt = ConstantInt::get(VarX->getType(), ShrAmt - ShlAmt);
> +      New = BinaryOperator::CreateLShr(VarX, Amt);
> +    }
> +
> +    return InsertNewInstWith(New, *Shl);
> +  }
> +
> +  return 0;
> +}
> 
> /// SimplifyDemandedVectorElts - The specified value produces a vector with
> /// any number of elements. DemandedElts contains the set of elements that are
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/2010-11-01-lshr-mask.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2010-11-01-lshr-mask.ll?rev=169182&r1=169181&r2=169182&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/2010-11-01-lshr-mask.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/2010-11-01-lshr-mask.ll Mon Dec  3 18:04:54 2012
> @@ -5,8 +5,8 @@
> define i32 @main(i32 %argc) nounwind ssp {
> entry:
>   %tmp3151 = trunc i32 %argc to i8
> -; CHECK: %tmp3163 = shl i8 %tmp3162, 6
> -; CHECK: and i8 %tmp3163, 64
> +; CHECK: %tmp3162 = shl i8 %tmp3151, 5
> +; CHECK: and i8 %tmp3162, 64
> ; CHECK-NOT: shl
> ; CHECK-NOT: shr
>   %tmp3161 = or i8 %tmp3151, -17
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/shift.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/shift.ll?rev=169182&r1=169181&r2=169182&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/shift.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/shift.ll Mon Dec  3 18:04:54 2012
> @@ -659,3 +659,79 @@
> ; CHECK-NEXT: %B = shl nuw i32 %x, 2
> ; CHECK-NEXT: ret i32 %B
> }
> +
> +define i32 @test54(i32 %x) {
> +  %shr2 = lshr i32 %x, 1
> +  %shl = shl i32 %shr2, 4
> +  %and = and i32 %shl, 16
> +  ret i32 %and
> +; CHECK: @test54
> +; CHECK: shl i32 %x, 3
> +}
> +
> +
> +define i32 @test55(i32 %x) {
> +  %shr2 = lshr i32 %x, 1
> +  %shl = shl i32 %shr2, 4
> +  %or = or i32 %shl, 8
> +  ret i32 %or
> +; CHECK: @test55
> +; CHECK: shl i32 %x, 3
> +}
> +
> +define i32 @test56(i32 %x) {
> +  %shr2 = lshr i32 %x, 1
> +  %shl = shl i32 %shr2, 4
> +  %or = or i32 %shl, 7
> +  ret i32 %or
> +; CHECK: @test56
> +; CHECK: shl i32 %shr2, 4
> +}
> +
> +
> +define i32 @test57(i32 %x) {
> +  %shr = lshr i32 %x, 1
> +  %shl = shl i32 %shr, 4
> +  %and = and i32 %shl, 16
> +  ret i32 %and
> +; CHECK: @test57
> +; CHECK: shl i32 %x, 3
> +}
> +
> +define i32 @test58(i32 %x) {
> +  %shr = lshr i32 %x, 1
> +  %shl = shl i32 %shr, 4
> +  %or = or i32 %shl, 8
> +  ret i32 %or
> +; CHECK: @test58
> +; CHECK: shl i32 %x, 3
> +}
> +
> +define i32 @test59(i32 %x) {
> +  %shr = ashr i32 %x, 1
> +  %shl = shl i32 %shr, 4
> +  %or = or i32 %shl, 7
> +  ret i32 %or
> +; CHECK: @test59
> +; CHECK: %shl = shl i32 %shr1, 4
> +}
> +
> +
> +define i32 @test60(i32 %x) {
> +  %shr = ashr i32 %x, 4
> +  %shl = shl i32 %shr, 1
> +  %or = or i32 %shl, 1
> +  ret i32 %or
> +; CHECK: @test60
> +; CHECK: lshr i32 %x, 3
> +}
> +
> +
> +define i32 @test61(i32 %x) {
> +  %shr = ashr i32 %x, 4
> +  %shl = shl i32 %shr, 1
> +  %or = or i32 %shl, 2
> +  ret i32 %or
> +; CHECK: @test61
> +; CHECK: ashr i32 %x, 4
> +}
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/signext.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/signext.ll?rev=169182&r1=169181&r2=169182&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/signext.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/signext.ll Mon Dec  3 18:04:54 2012
> @@ -82,6 +82,6 @@
>   %sub = add i32 %xor, -67108864                  ; <i32> [#uses=1]
>   ret i32 %sub
> ; CHECK: @test8
> -; CHECK: %shr = ashr i32 %x, 5
> -; CHECK: ret i32 %shr
> +; CHECK: %sub = ashr i32 %x, 5
> +; CHECK: ret i32 %sub
> }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list