[llvm] d6f0600 - [NFC][InstCombine] Add FIXME's for getLogBase2() / visitUDivOperand()

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2020-08-12T22:06:54+03:00
New Revision: d6f0600c96a6b05ccfe378c9ab9dc0d426f92bd4

URL: https://github.com/llvm/llvm-project/commit/d6f0600c96a6b05ccfe378c9ab9dc0d426f92bd4
DIFF: https://github.com/llvm/llvm-project/commit/d6f0600c96a6b05ccfe378c9ab9dc0d426f92bd4.diff

LOG: [NFC][InstCombine] Add FIXME's for getLogBase2() / visitUDivOperand()

These are not correctness issues.

In visitUDivOperand(), if the (potential) divisor is undef, then udiv is
already UB, so it is not incorrect to keep undef as shift amount.

But, that is suboptimal.
We could instead simply drop that select, picking the other operand.

Afterwards, getLogBase2() could assert that there is no undef in divisor.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
index 6162c546ff10..203c8c7f1c1b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -102,6 +102,9 @@ static Value *simplifyValueKnownNonZero(Value *V, InstCombinerImpl &IC,
 /// of C.
 /// Return a null pointer otherwise.
 static Constant *getLogBase2(Type *Ty, Constant *C) {
+  // Note that log2(iN undef) is *NOT* iN undef, because log2(iN undef) u< N.
+  // FIXME: just assert that C there is no undef in \p C.
+
   const APInt *IVal;
   if (match(C, m_APInt(IVal)) && IVal->isPowerOf2())
     return ConstantInt::get(Ty, IVal->logBase2());
@@ -933,6 +936,8 @@ static Instruction *foldUDivShl(Value *Op0, Value *Op1, const BinaryOperator &I,
 static size_t visitUDivOperand(Value *Op0, Value *Op1, const BinaryOperator &I,
                                SmallVectorImpl<UDivFoldAction> &Actions,
                                unsigned Depth = 0) {
+  // FIXME: assert that Op1 isn't/doesn't contain undef.
+
   // Check to see if this is an unsigned division with an exact power of 2,
   // if so, convert to a right shift.
   if (match(Op1, m_Power2())) {
@@ -952,6 +957,9 @@ static size_t visitUDivOperand(Value *Op0, Value *Op1, const BinaryOperator &I,
     return 0;
 
   if (SelectInst *SI = dyn_cast<SelectInst>(Op1))
+    // FIXME: missed optimization: if one of the hands of select is/contains
+    //        undef, just directly pick the other one.
+    // FIXME: can both hands contain undef?
     if (size_t LHSIdx =
             visitUDivOperand(Op0, SI->getOperand(1), I, Actions, Depth))
       if (visitUDivOperand(Op0, SI->getOperand(2), I, Actions, Depth)) {