[ValueTracking] Slight refactor to avoid unnecessary work; NFC

[goldstein.w.n]

Summary

Two changes:

1. Make some cases that conditionally returned unconditional.
2. In cases of Op0 != 0 || Op1 != 0 its better check Op1 first as its more likely to be a constant due to canonicalization (so faster to get knownbits of).

[nikic]

In cases of Op0 != 0 || Op1 != 0 its better check Op1 first as its more likely to be a constant due to canonicalization (so faster to get knownbits of).

I'm not sure this is right: If Op1 is constant, then isKnownNonZero will in practice always return true, because the zero cases get folded away. So we do the call on Op0 anyway.

[nikic]

llvm/lib/Analysis/ValueTracking.cpp

2916

This one should probably also be changed to return directly?

[goldstein.w.n]

In D149419#4304516, @nikic wrote:

In cases of Op0 != 0 || Op1 != 0 its better check Op1 first as its more likely to be a constant due to canonicalization (so faster to get knownbits of).

I'm not sure this is right: If Op1 is constant, then isKnownNonZero will in practice always return true, because the zero cases get folded away. So we do the call on Op0 anyway.

Hmm? In something like the Or case if we have or A, 12, isKnownNonZero(Op1) || isKnownOpZero(Op0) will return true after just finding the constant 12. If you flip the order
it first need to recursve through the A case which can be be alot more expensive.

[nikic]

In D149419#4306616, @goldstein.w.n wrote:

In D149419#4304516, @nikic wrote:

In cases of Op0 != 0 || Op1 != 0 its better check Op1 first as its more likely to be a constant due to canonicalization (so faster to get knownbits of).

I'm not sure this is right: If Op1 is constant, then isKnownNonZero will in practice always return true, because the zero cases get folded away. So we do the call on Op0 anyway.

Hmm? In something like the Or case if we have or A, 12, isKnownNonZero(Op1) || isKnownOpZero(Op0) will return true after just finding the constant 12. If you flip the order
it first need to recursve through the A case which can be be alot more expensive.

Oh yeah, you're right. Ignore what I said there. I was thinking of the && case, not the || case.

[nikic]

LGTM

[goldstein.w.n]

Add missing case.

[goldstein.w.n]

Rebase

[goldstein.w.n]

Rebase

[goldstein.w.n]

Rebase

llvm/lib/Analysis/ValueTracking.cpp

if (!isa<ScalableVectorType>(I->getType()) &&
             Q.DL.getTypeSizeInBits(I->getType()).getFixedValue())
       return isKnownNonZero(I->getOperand(0), Depth, Q);
     break;
   case Instruction::Sub:
     return isNonZeroSub(DemandedElts, Depth, Q, BitWidth, I->getOperand(0),
                         I->getOperand(1));
   case Instruction::Or:
     // X | Y != 0 if X != 0 or Y != 0.
-    return isKnownNonZero(I->getOperand(0), DemandedElts, Depth, Q) ||
-           isKnownNonZero(I->getOperand(1), DemandedElts, Depth, Q);
+    return isKnownNonZero(I->getOperand(1), DemandedElts, Depth, Q) ||
+           isKnownNonZero(I->getOperand(0), DemandedElts, Depth, Q);
   case Instruction::SExt:
   case Instruction::ZExt:
     // ext X != 0 if X != 0.
     return isKnownNonZero(I->getOperand(0), Depth, Q);
 
   case Instruction::Shl: {
     // shl nsw/nuw can't remove any non-zero bits.
     const OverflowingBinaryOperator *BO = cast<OverflowingBinaryOperator>(V);
case Instruction::SDiv:
     break;
   case Instruction::Add: {
     // X + Y.
 
     // If Add has nuw wrap flag, then if either X or Y is non-zero the result is
     // non-zero.
     auto *BO = cast<OverflowingBinaryOperator>(V);
     if (Q.IIQ.hasNoUnsignedWrap(BO))
-      return isKnownNonZero(I->getOperand(0), DemandedElts, Depth, Q) ||
-             isKnownNonZero(I->getOperand(1), DemandedElts, Depth, Q);
+      return isKnownNonZero(I->getOperand(1), DemandedElts, Depth, Q) ||
+             isKnownNonZero(I->getOperand(0), DemandedElts, Depth, Q);
 
     if (isNonZeroAdd(DemandedElts, Depth, Q, BitWidth, I->getOperand(0),
                      I->getOperand(1)))
       return true;
     break;
   }
   case Instruction::Mul: {
     // If X and Y are non-zero then so is X * Y as long as the multiplication
if (auto *II = dyn_cast<IntrinsicInst>(I)) {
       switch (II->getIntrinsicID()) {
       case Intrinsic::sshl_sat:
       case Intrinsic::ushl_sat:
       case Intrinsic::abs:
       case Intrinsic::bitreverse:
       case Intrinsic::bswap:
       case Intrinsic::ctpop:
         if (isKnownNonZero(II->getArgOperand(0), DemandedElts, Depth, Q))
           return true;
         break;
       case Intrinsic::ssub_sat:
         return isNonZeroSub(DemandedElts, Depth, Q, BitWidth,
                             II->getArgOperand(0), II->getArgOperand(1));
       case Intrinsic::usub_sat: {
         KnownBits XKnown =
             computeKnownBits(II->getArgOperand(0), DemandedElts, Depth, Q);
         if (XKnown.isUnknown())
           return false;
         KnownBits YKnown =
             computeKnownBits(II->getArgOperand(1), DemandedElts, Depth, Q);
         std::optional<bool> ugt = KnownBits::ugt(XKnown, YKnown);
         return ugt && *ugt;
       }
       case Intrinsic::sadd_sat:
         if (isNonZeroAdd(DemandedElts, Depth, Q, BitWidth, II->getArgOperand(0),
                          II->getArgOperand(1)))
           return true;
         break;
       case Intrinsic::umax:
       case Intrinsic::uadd_sat:
-        if (isKnownNonZero(II->getArgOperand(0), DemandedElts, Depth, Q) ||
-            isKnownNonZero(II->getArgOperand(1), DemandedElts, Depth, Q))
-          return true;
-        break;
+        return isKnownNonZero(II->getArgOperand(1), DemandedElts, Depth, Q) ||
+               isKnownNonZero(II->getArgOperand(0), DemandedElts, Depth, Q);
       case Intrinsic::smin:
       case Intrinsic::smax: {
         auto KnownOpImpliesNonZero = [&](const KnownBits &K) {
           return II->getIntrinsicID() == Intrinsic::smin
                      ? K.isNegative()
                      : K.isStrictlyPositive();
         };
         KnownBits XKnown =