[InstCombine] Remove one use restriction on the shift for calls to foldICmpAndShift.

[craig.topper]

Summary

If this transformation succeeds, we're going to remove our dependency on the shift by rewriting the and. So it doesn't matter how many uses the shift has.

This distributes the one use check to other transforms in foldICmpAndConstConst that do need it.

[spatel]

lib/Transforms/InstCombine/InstCombineCompares.cpp

1717–1718

We're also easing the restriction on this fold, right? Add another test for it too?

[spatel]

LGTM. The transform that I asked about no longer exists after rL314153, so that question is decoupled from this patch. Still might be worth adding a test if one doesn't exist already.

lib/Transforms/InstCombine/InstCombineCompares.cpp

 /// Fold icmp (and X, C2), C1.
 Instruction *InstCombiner::foldICmpAndConstConst(ICmpInst &Cmp,
                                                  BinaryOperator *And,
                                                  const APInt *C1) {
   const APInt *C2;
   if (!match(And->getOperand(1), m_APInt(C2)))
     return nullptr;
 
-  if (!And->hasOneUse() || !And->getOperand(0)->hasOneUse())
+  if (!And->hasOneUse())
     return nullptr;
 
   // If the LHS is an 'and' of a truncate and we can widen the and/compare to
   // the input width without changing the value produced, eliminate the cast:
   //
   // icmp (and (trunc W), C2), C1 -> icmp (and W, C2'), C1'
   //
   // We can do this transformation if the constants do not have their sign bits
   // set or if it is an equality comparison. Extending a relational comparison
   // when we're checking the sign bit would not work.
   Value *W;
-  if (match(And->getOperand(0), m_Trunc(m_Value(W))) &&
+  if (match(And->getOperand(0), m_OneUse(m_Trunc(m_Value(W)))) &&
       (Cmp.isEquality() || (!C1->isNegative() && !C2->isNegative()))) {
     // TODO: Is this a good transform for vectors? Wider types may reduce
     // throughput. Should this transform be limited (even for scalars) by using
     // shouldChangeType()?
     if (!Cmp.getType()->isVectorTy()) {
       Type *WideType = W->getType();
       unsigned WideScalarBits = WideType->getScalarSizeInBits();
       Constant *ZextC1 = ConstantInt::get(WideType, C1->zext(WideScalarBits));
       Constant *ZextC2 = ConstantInt::get(WideType, C2->zext(WideScalarBits));
       Value *NewAnd = Builder.CreateAnd(W, ZextC2, And->getName());
       return new ICmpInst(Cmp.getPredicate(), NewAnd, ZextC1);
     }
   }
 
   if (Instruction *I = foldICmpAndShift(Cmp, And, C1, C2))
     return I;
 
   // (icmp pred (and (or (lshr A, B), A), 1), 0) -->
   // (icmp pred (and A, (or (shl 1, B), 1), 0))
   //
   // iff pred isn't signed
-  if (!Cmp.isSigned() && C1->isNullValue() &&
+  if (!Cmp.isSigned() && C1->isNullValue() && And->getOperand(0)->hasOneUse() &&
       match(And->getOperand(1), m_One())) {
     Constant *One = cast<Constant>(And->getOperand(1));
     Value *Or = And->getOperand(0);
     Value *A, *B, *LShr;
     if (match(Or, m_Or(m_Value(LShr), m_Value(A))) &&
         match(LShr, m_LShr(m_Specific(A), m_Value(B)))) {
       unsigned UsesRemoved = 0;
       if (And->hasOneUse())
if (match(Or, m_Or(m_Value(LShr), m_Value(A))) &&
       if (NewOr) {
         Value *NewAnd = Builder.CreateAnd(A, NewOr, And->getName());
         Cmp.setOperand(0, NewAnd);
         return &Cmp;
       }
     }
   }
 
   // (X & C2) > C1 --> (X & C2) != 0, if any bit set in (X & C2) will produce a
  // result greater than C1. Also handle (X & C2) < C1 --> (X & C2) == 0.
   if (!C2->isNullValue()) {
     unsigned NumTZ = C2->countTrailingZeros();
     if (Cmp.getPredicate() == ICmpInst::ICMP_UGT &&
         NumTZ >= C1->getActiveBits()) {
       Constant *Zero = Constant::getNullValue(And->getType());
       return new ICmpInst(ICmpInst::ICMP_NE, And, Zero);
     }
     if (Cmp.getPredicate() == ICmpInst::ICMP_ULT &&

test/Transforms/InstCombine/icmp.ll

 ;
   %shr = lshr <2 x i32> %X, <i32 4, i32 4>
   %and = and <2 x i32> %shr, <i32 15, i32 15>
   %add = add <2 x i32> %and, <i32 -14, i32 -14>
   %tobool = icmp ne <2 x i32> %add, zeroinitializer
   ret <2 x i1> %tobool
 }
 
+; Variation of the above with an extra use of the shift
+define i1 @icmp_and_shr_multiuse(i32 %X) {
+; CHECK-LABEL: @icmp_and_shr_multiuse(
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[X:%.*]], 240
+; CHECK-NEXT:    [[AND2:%.*]] = and i32 [[X]], 496
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp ne i32 [[AND]], 224
+; CHECK-NEXT:    [[TOBOOL2:%.*]] = icmp ne i32 [[AND2]], 432
+; CHECK-NEXT:    [[AND3:%.*]] = and i1 [[TOBOOL]], [[TOBOOL2]]
+; CHECK-NEXT:    ret i1 [[AND3]]
+;
+  %shr = lshr i32 %X, 4
+  %and = and i32 %shr, 15
+  %and2 = and i32 %shr, 31 ; second use of the shift
+  %tobool = icmp ne i32 %and, 14
+  %tobool2 = icmp ne i32 %and2, 27
+  %and3 = and i1 %tobool, %tobool2
+  ret i1 %and3
+}
+
 ; PR16244
 define i1 @test71(i8* %x) {
 ; CHECK-LABEL: @test71(
 ; CHECK-NEXT:    ret i1 false
 ;
   %a = getelementptr i8, i8* %x, i64 8
   %b = getelementptr inbounds i8, i8* %x, i64 8
   %c = icmp ugt i8* %a, %b