[llvm-commits] [PATCH, instcombine] shift-right + shift-left => single shift

Shuxin Yang shuxin.llvm at gmail.com
Mon Dec 3 10:59:21 PST 2012


Hi,

    For rdar://12329730, part 2.

    This patch tries to simplify E1 = " X >> C1 << C2", along with 
demanded bitmask,
into "E2 = "X << (C2 - C1)" or "E2 = X >> (C1 - C2)", depending on the 
sign of C1 - C2.

    In general E1 and E2 are not equal. Suppose they may diff in the 
bits S={bm,..., bn}
regardless the value X is holding.

    This transformation is legal iff one of the following conditions is 
hold:

=====================================================
  a) We don't care the bits in S,  per the input the demanded bitmask. i.e.
     DemanedBitmask & E1 == DemandedBitmask & E2, regardless the value X 
is having.

  b) X is holding special value such that E1 == E2.

  c) hybrid of a) and b). Some bits in S are equal, thanks to the 
special value hold by
    X, and we don't care the rest bits in S.
======================================================

   It is bit expensive to test b), and I don't know how useful it will be.
   c) is much more involved.

   This change only test if condition a) is satisfied.


  The change to test/Transforms/InstCombine/2010-11-01-lshr-mask.ll 
doesn't seem to be
very obvious.  I compare the WAS/IS in the following. As you can see, 
this xform reduce
one instruction.


   1 2010-11-01-lshr-mask.ll:
   2
   3 IS:
   4 ==
   5 define i32 @main(i32 %argc) nounwind readnone ssp {
   6 entry:
   7   %tmp3151 = trunc i32 %argc to i8
   8   %tmp3162 = shl i8 %tmp3151, 5
   9   %tmp3163 = and i8 %tmp3162, 64
  10   %tmp4127 = xor i8 %tmp3163, 64
  11   %tmp4086 = zext i8 %tmp4127 to i32
  12   ret i32 %tmp4086
  13 }
  14
  15
  16 WAS:
  17 define i32 @main(i32 %argc) nounwind readnone ssp {
  18 entry:
  19   %tmp3151 = trunc i32 %argc to i8
  20   %tmp3162 = lshr i8 %tmp3151, 1
  21   %tmp3163 = shl i8 %tmp3162, 6
  22   %0 = and i8 %tmp3163, 64
  23   %tmp4127 = xor i8 %0, 64
  24   %tmp4086 = zext i8 %tmp4127 to i32
  25   ret i32 %tmp4086
  26 }


Thank you for review.
Shuxin



-------------- next part --------------
Index: test/Transforms/InstCombine/signext.ll
===================================================================
--- test/Transforms/InstCombine/signext.ll	(revision 169135)
+++ test/Transforms/InstCombine/signext.ll	(working copy)
@@ -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
 }
Index: test/Transforms/InstCombine/shift.ll
===================================================================
--- test/Transforms/InstCombine/shift.ll	(revision 169135)
+++ test/Transforms/InstCombine/shift.ll	(working copy)
@@ -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
+}
Index: test/Transforms/InstCombine/2010-11-01-lshr-mask.ll
===================================================================
--- test/Transforms/InstCombine/2010-11-01-lshr-mask.ll	(revision 169135)
+++ test/Transforms/InstCombine/2010-11-01-lshr-mask.ll	(working copy)
@@ -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
Index: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp	(revision 169135)
+++ lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp	(working copy)
@@ -16,10 +16,11 @@
 #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
 /// are any bits set in the constant that are not demanded.  If so, shrink the
@@ -580,6 +581,16 @@
     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,7 +811,79 @@
   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
 /// actually used by the caller.  This method analyzes which elements of the
Index: lib/Transforms/InstCombine/InstCombine.h
===================================================================
--- lib/Transforms/InstCombine/InstCombine.h	(revision 169135)
+++ lib/Transforms/InstCombine/InstCombine.h	(working copy)
@@ -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


More information about the llvm-commits mailing list