From d6f0600c96a6b05ccfe378c9ab9dc0d426f92bd4 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Wed, 12 Aug 2020 21:59:57 +0300 Subject: [PATCH] [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. --- llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) 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 &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(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)) {