[PATCH] D23134: Make cltz and cttz zero undef when the operand cannot be zero in InstCombine

[Amaury SECHET via llvm-commits]

deadalnix added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:1414-1452
@@ -1399,37 +1413,41 @@
     break;
   case Intrinsic::cttz: {
     // If all bits below the first known one are known zero,
     // this value is constant.
-    IntegerType *IT = dyn_cast<IntegerType>(II->getArgOperand(0)->getType());
+    Value *Op0 = II->getArgOperand(0);
+    IntegerType *IT = dyn_cast<IntegerType>(Op0->getType());
     // FIXME: Try to simplify vectors of integers.
     if (!IT) break;
     uint32_t BitWidth = IT->getBitWidth();
     APInt KnownZero(BitWidth, 0);
     APInt KnownOne(BitWidth, 0);
-    computeKnownBits(II->getArgOperand(0), KnownZero, KnownOne, 0, II);
+    computeKnownBits(Op0, KnownZero, KnownOne, 0, II);
     unsigned TrailingZeros = KnownOne.countTrailingZeros();
     APInt Mask(APInt::getLowBitsSet(BitWidth, TrailingZeros));
     if ((Mask & KnownZero) == Mask)
       return replaceInstUsesWith(CI, ConstantInt::get(IT,
                                  APInt(BitWidth, TrailingZeros)));
-
+    if (KnownOne != 0 || isKnownNonZero(Op0, DL))
+      makeCttzCtlzZeroUndef(*this, II);
     }
     break;
   case Intrinsic::ctlz: {
     // If all bits above the first known one are known zero,
     // this value is constant.
-    IntegerType *IT = dyn_cast<IntegerType>(II->getArgOperand(0)->getType());
+    Value *Op0 = II->getArgOperand(0);
+    IntegerType *IT = dyn_cast<IntegerType>(Op0->getType());
     // FIXME: Try to simplify vectors of integers.
     if (!IT) break;
     uint32_t BitWidth = IT->getBitWidth();
     APInt KnownZero(BitWidth, 0);
     APInt KnownOne(BitWidth, 0);
-    computeKnownBits(II->getArgOperand(0), KnownZero, KnownOne, 0, II);
+    computeKnownBits(Op0, KnownZero, KnownOne, 0, II);
     unsigned LeadingZeros = KnownOne.countLeadingZeros();
     APInt Mask(APInt::getHighBitsSet(BitWidth, LeadingZeros));
     if ((Mask & KnownZero) == Mask)
       return replaceInstUsesWith(CI, ConstantInt::get(IT,
                                  APInt(BitWidth, LeadingZeros)));
-
+    if (KnownOne != 0 || isKnownNonZero(Op0, DL))
+      makeCttzCtlzZeroUndef(*this, II);
     }
     break;
----------------
spatel wrote:
> I've drafted what I tried to suggest here as D23223. Let me know if that makes sense.
I don't think it really makes the code better. There are more branches and names become ambiguous as they can represent 2 things. Also, this is kind of out of the scope of this diff.


https://reviews.llvm.org/D23134