[InstCombine] Make takeLog2 usable throughout InstCombine; NFC
#122498
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This just encapsulates the common pattern:
```
if (takeLog2(..., /*DoFold=*/false)) {
Value * Log2 = takeLog2(..., /*DoFold=*/true);
...
}
```
|
@llvm/pr-subscribers-llvm-transforms Author: None (goldsteinn) Changes
Full diff: https://github.com/llvm/llvm-project/pull/122498.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
index fa6b60cba15aaf..213b1b73bed067 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
@@ -195,6 +195,18 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
PatternMatch::m_Value()));
}
+ // Take the exact integer log2 of the value. If DoFold is true, create the
+ // actual instructions, otherwise return a non-null dummy value. Return
+ // nullptr on failure. Note, if DoFold is true the caller must ensure that
+ // takeLog2 will succeed, otherwise it may create stray instructions.
+ Value *takeLog2(Value *Op, unsigned Depth, bool AssumeNonZero, bool DoFold);
+
+ Value *tryGetLog2(Value *Op, bool AssumeNonZero) {
+ if (takeLog2(Op, /*Depth=*/0, AssumeNonZero, /*DoFold=*/false))
+ return takeLog2(Op, /*Depth=*/0, AssumeNonZero, /*DoFold=*/true);
+ return nullptr;
+ }
+
/// Return nonnull value if V is free to invert under the condition of
/// WillInvertAllUses.
/// If Builder is nonnull, it will return a simplified ~V.
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
index 0c34cf01bdf1a9..e89a2c9579f422 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -185,9 +185,6 @@ static Value *foldMulShl1(BinaryOperator &Mul, bool CommuteOperands,
return nullptr;
}
-static Value *takeLog2(IRBuilderBase &Builder, Value *Op, unsigned Depth,
- bool AssumeNonZero, bool DoFold);
-
Instruction *InstCombinerImpl::visitMul(BinaryOperator &I) {
Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
if (Value *V =
@@ -531,19 +528,13 @@ Instruction *InstCombinerImpl::visitMul(BinaryOperator &I) {
// (shl Op1, Log2(Op0))
// if Log2(Op1) folds away ->
// (shl Op0, Log2(Op1))
- if (takeLog2(Builder, Op0, /*Depth*/ 0, /*AssumeNonZero*/ false,
- /*DoFold*/ false)) {
- Value *Res = takeLog2(Builder, Op0, /*Depth*/ 0, /*AssumeNonZero*/ false,
- /*DoFold*/ true);
+ if (Value *Res = tryGetLog2(Op0, /*AssumeNonZero=*/false)) {
BinaryOperator *Shl = BinaryOperator::CreateShl(Op1, Res);
// We can only propegate nuw flag.
Shl->setHasNoUnsignedWrap(HasNUW);
return Shl;
}
- if (takeLog2(Builder, Op1, /*Depth*/ 0, /*AssumeNonZero*/ false,
- /*DoFold*/ false)) {
- Value *Res = takeLog2(Builder, Op1, /*Depth*/ 0, /*AssumeNonZero*/ false,
- /*DoFold*/ true);
+ if (Value *Res = tryGetLog2(Op0, /*AssumeNonZero=*/false)) {
BinaryOperator *Shl = BinaryOperator::CreateShl(Op0, Res);
// We can only propegate nuw flag.
Shl->setHasNoUnsignedWrap(HasNUW);
@@ -1407,111 +1398,6 @@ Instruction *InstCombinerImpl::commonIDivTransforms(BinaryOperator &I) {
return nullptr;
}
-static const unsigned MaxDepth = 6;
-
-// Take the exact integer log2 of the value. If DoFold is true, create the
-// actual instructions, otherwise return a non-null dummy value. Return nullptr
-// on failure.
-static Value *takeLog2(IRBuilderBase &Builder, Value *Op, unsigned Depth,
- bool AssumeNonZero, bool DoFold) {
- auto IfFold = [DoFold](function_ref<Value *()> Fn) {
- if (!DoFold)
- return reinterpret_cast<Value *>(-1);
- return Fn();
- };
-
- // FIXME: assert that Op1 isn't/doesn't contain undef.
-
- // log2(2^C) -> C
- if (match(Op, m_Power2()))
- return IfFold([&]() {
- Constant *C = ConstantExpr::getExactLogBase2(cast<Constant>(Op));
- if (!C)
- llvm_unreachable("Failed to constant fold udiv -> logbase2");
- return C;
- });
-
- // The remaining tests are all recursive, so bail out if we hit the limit.
- if (Depth++ == MaxDepth)
- return nullptr;
-
- // log2(zext X) -> zext log2(X)
- // FIXME: Require one use?
- Value *X, *Y;
- if (match(Op, m_ZExt(m_Value(X))))
- if (Value *LogX = takeLog2(Builder, X, Depth, AssumeNonZero, DoFold))
- return IfFold([&]() { return Builder.CreateZExt(LogX, Op->getType()); });
-
- // log2(trunc x) -> trunc log2(X)
- // FIXME: Require one use?
- if (match(Op, m_Trunc(m_Value(X)))) {
- auto *TI = cast<TruncInst>(Op);
- if (AssumeNonZero || TI->hasNoUnsignedWrap())
- if (Value *LogX = takeLog2(Builder, X, Depth, AssumeNonZero, DoFold))
- return IfFold([&]() {
- return Builder.CreateTrunc(LogX, Op->getType(), "",
- /*IsNUW=*/TI->hasNoUnsignedWrap());
- });
- }
-
- // log2(X << Y) -> log2(X) + Y
- // FIXME: Require one use unless X is 1?
- if (match(Op, m_Shl(m_Value(X), m_Value(Y)))) {
- auto *BO = cast<OverflowingBinaryOperator>(Op);
- // nuw will be set if the `shl` is trivially non-zero.
- if (AssumeNonZero || BO->hasNoUnsignedWrap() || BO->hasNoSignedWrap())
- if (Value *LogX = takeLog2(Builder, X, Depth, AssumeNonZero, DoFold))
- return IfFold([&]() { return Builder.CreateAdd(LogX, Y); });
- }
-
- // log2(X >>u Y) -> log2(X) - Y
- // FIXME: Require one use?
- if (match(Op, m_LShr(m_Value(X), m_Value(Y)))) {
- auto *PEO = cast<PossiblyExactOperator>(Op);
- if (AssumeNonZero || PEO->isExact())
- if (Value *LogX = takeLog2(Builder, X, Depth, AssumeNonZero, DoFold))
- return IfFold([&]() { return Builder.CreateSub(LogX, Y); });
- }
-
- // log2(X & Y) -> either log2(X) or log2(Y)
- // This requires `AssumeNonZero` as `X & Y` may be zero when X != Y.
- if (AssumeNonZero && match(Op, m_And(m_Value(X), m_Value(Y)))) {
- if (Value *LogX = takeLog2(Builder, X, Depth, AssumeNonZero, DoFold))
- return IfFold([&]() { return LogX; });
- if (Value *LogY = takeLog2(Builder, Y, Depth, AssumeNonZero, DoFold))
- return IfFold([&]() { return LogY; });
- }
-
- // log2(Cond ? X : Y) -> Cond ? log2(X) : log2(Y)
- // FIXME: Require one use?
- if (SelectInst *SI = dyn_cast<SelectInst>(Op))
- if (Value *LogX = takeLog2(Builder, SI->getOperand(1), Depth,
- AssumeNonZero, DoFold))
- if (Value *LogY = takeLog2(Builder, SI->getOperand(2), Depth,
- AssumeNonZero, DoFold))
- return IfFold([&]() {
- return Builder.CreateSelect(SI->getOperand(0), LogX, LogY);
- });
-
- // log2(umin(X, Y)) -> umin(log2(X), log2(Y))
- // log2(umax(X, Y)) -> umax(log2(X), log2(Y))
- auto *MinMax = dyn_cast<MinMaxIntrinsic>(Op);
- if (MinMax && MinMax->hasOneUse() && !MinMax->isSigned()) {
- // Use AssumeNonZero as false here. Otherwise we can hit case where
- // log2(umax(X, Y)) != umax(log2(X), log2(Y)) (because overflow).
- if (Value *LogX = takeLog2(Builder, MinMax->getLHS(), Depth,
- /*AssumeNonZero*/ false, DoFold))
- if (Value *LogY = takeLog2(Builder, MinMax->getRHS(), Depth,
- /*AssumeNonZero*/ false, DoFold))
- return IfFold([&]() {
- return Builder.CreateBinaryIntrinsic(MinMax->getIntrinsicID(), LogX,
- LogY);
- });
- }
-
- return nullptr;
-}
-
/// If we have zero-extended operands of an unsigned div or rem, we may be able
/// to narrow the operation (sink the zext below the math).
static Instruction *narrowUDivURem(BinaryOperator &I,
@@ -1614,13 +1500,9 @@ Instruction *InstCombinerImpl::visitUDiv(BinaryOperator &I) {
}
// Op1 udiv Op2 -> Op1 lshr log2(Op2), if log2() folds away.
- if (takeLog2(Builder, Op1, /*Depth*/ 0, /*AssumeNonZero*/ true,
- /*DoFold*/ false)) {
- Value *Res = takeLog2(Builder, Op1, /*Depth*/ 0,
- /*AssumeNonZero*/ true, /*DoFold*/ true);
+ if (Value *Res = tryGetLog2(Op1, /*AssumeNonZero=*/true))
return replaceInstUsesWith(
I, Builder.CreateLShr(Op0, Res, I.getName(), I.isExact()));
- }
return nullptr;
}
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 2fb60ef11499c7..ede7aa48cd6401 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2565,6 +2565,105 @@ Instruction *InstCombinerImpl::visitGEPOfGEP(GetElementPtrInst &GEP,
return nullptr;
}
+Value *InstCombiner::takeLog2(Value *Op, unsigned Depth, bool AssumeNonZero,
+ bool DoFold) {
+ auto IfFold = [DoFold](function_ref<Value *()> Fn) {
+ if (!DoFold)
+ return reinterpret_cast<Value *>(-1);
+ return Fn();
+ };
+
+ // FIXME: assert that Op1 isn't/doesn't contain undef.
+
+ // log2(2^C) -> C
+ if (match(Op, m_Power2()))
+ return IfFold([&]() {
+ Constant *C = ConstantExpr::getExactLogBase2(cast<Constant>(Op));
+ if (!C)
+ llvm_unreachable("Failed to constant fold udiv -> logbase2");
+ return C;
+ });
+
+ // The remaining tests are all recursive, so bail out if we hit the limit.
+ if (Depth++ == MaxAnalysisRecursionDepth)
+ return nullptr;
+
+ // log2(zext X) -> zext log2(X)
+ // FIXME: Require one use?
+ Value *X, *Y;
+ if (match(Op, m_ZExt(m_Value(X))))
+ if (Value *LogX = takeLog2(X, Depth, AssumeNonZero, DoFold))
+ return IfFold([&]() { return Builder.CreateZExt(LogX, Op->getType()); });
+
+ // log2(trunc x) -> trunc log2(X)
+ // FIXME: Require one use?
+ if (match(Op, m_Trunc(m_Value(X)))) {
+ auto *TI = cast<TruncInst>(Op);
+ if (AssumeNonZero || TI->hasNoUnsignedWrap())
+ if (Value *LogX = takeLog2(X, Depth, AssumeNonZero, DoFold))
+ return IfFold([&]() {
+ return Builder.CreateTrunc(LogX, Op->getType(), "",
+ /*IsNUW=*/TI->hasNoUnsignedWrap());
+ });
+ }
+
+ // log2(X << Y) -> log2(X) + Y
+ // FIXME: Require one use unless X is 1?
+ if (match(Op, m_Shl(m_Value(X), m_Value(Y)))) {
+ auto *BO = cast<OverflowingBinaryOperator>(Op);
+ // nuw will be set if the `shl` is trivially non-zero.
+ if (AssumeNonZero || BO->hasNoUnsignedWrap() || BO->hasNoSignedWrap())
+ if (Value *LogX = takeLog2(X, Depth, AssumeNonZero, DoFold))
+ return IfFold([&]() { return Builder.CreateAdd(LogX, Y); });
+ }
+
+ // log2(X >>u Y) -> log2(X) - Y
+ // FIXME: Require one use?
+ if (match(Op, m_LShr(m_Value(X), m_Value(Y)))) {
+ auto *PEO = cast<PossiblyExactOperator>(Op);
+ if (AssumeNonZero || PEO->isExact())
+ if (Value *LogX = takeLog2(X, Depth, AssumeNonZero, DoFold))
+ return IfFold([&]() { return Builder.CreateSub(LogX, Y); });
+ }
+
+ // log2(X & Y) -> either log2(X) or log2(Y)
+ // This requires `AssumeNonZero` as `X & Y` may be zero when X != Y.
+ if (AssumeNonZero && match(Op, m_And(m_Value(X), m_Value(Y)))) {
+ if (Value *LogX = takeLog2(X, Depth, AssumeNonZero, DoFold))
+ return IfFold([&]() { return LogX; });
+ if (Value *LogY = takeLog2(Y, Depth, AssumeNonZero, DoFold))
+ return IfFold([&]() { return LogY; });
+ }
+
+ // log2(Cond ? X : Y) -> Cond ? log2(X) : log2(Y)
+ // FIXME: Require one use?
+ if (SelectInst *SI = dyn_cast<SelectInst>(Op))
+ if (Value *LogX = takeLog2(SI->getOperand(1), Depth, AssumeNonZero, DoFold))
+ if (Value *LogY =
+ takeLog2(SI->getOperand(2), Depth, AssumeNonZero, DoFold))
+ return IfFold([&]() {
+ return Builder.CreateSelect(SI->getOperand(0), LogX, LogY);
+ });
+
+ // log2(umin(X, Y)) -> umin(log2(X), log2(Y))
+ // log2(umax(X, Y)) -> umax(log2(X), log2(Y))
+ auto *MinMax = dyn_cast<MinMaxIntrinsic>(Op);
+ if (MinMax && MinMax->hasOneUse() && !MinMax->isSigned()) {
+ // Use AssumeNonZero as false here. Otherwise we can hit case where
+ // log2(umax(X, Y)) != umax(log2(X), log2(Y)) (because overflow).
+ if (Value *LogX = takeLog2(MinMax->getLHS(), Depth,
+ /*AssumeNonZero*/ false, DoFold))
+ if (Value *LogY = takeLog2(MinMax->getRHS(), Depth,
+ /*AssumeNonZero*/ false, DoFold))
+ return IfFold([&]() {
+ return Builder.CreateBinaryIntrinsic(MinMax->getIntrinsicID(), LogX,
+ LogY);
+ });
+ }
+
+ return nullptr;
+}
+
Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
BuilderTy *Builder,
bool &DoesConsume, unsigned Depth) {
|
goldsteinn
changed the title
takeLog2 usable throughout InstCombine; NFC
|
✅ With the latest revision this PR passed the undef deprecator. |
goldsteinn
commented
Jan 10, 2025
| /*DoFold*/ false)) { | ||
| Value *Res = takeLog2(Builder, Op1, /*Depth*/ 0, /*AssumeNonZero*/ false, | ||
| /*DoFold*/ true); | ||
| if (Value *Res = tryGetLog2(Op0, /*AssumeNonZero=*/false)) { |
nikic
reviewed
Jan 10, 2025
nikic
reviewed
Jan 10, 2025
nikic
approved these changes
Jan 10, 2025
Comment on lines
+789
to
+792
| // Take the exact integer log2 of the value. If DoFold is true, create the | ||
| // actual instructions, otherwise return a non-null dummy value. Return | ||
| // nullptr on failure. Note, if DoFold is true the caller must ensure that | ||
| // takeLog2 will succeed, otherwise it may create stray instructions. |
There was a problem hiding this comment.
Suggested change
| // Take the exact integer log2 of the value. If DoFold is true, create the | |
| // actual instructions, otherwise return a non-null dummy value. Return | |
| // nullptr on failure. Note, if DoFold is true the caller must ensure that | |
| // takeLog2 will succeed, otherwise it may create stray instructions. | |
| /// Take the exact integer log2 of the value. If DoFold is true, create the | |
| /// actual instructions, otherwise return a non-null dummy value. Return | |
| /// nullptr on failure. Note, if DoFold is true the caller must ensure that | |
| /// takeLog2 will succeed, otherwise it may create stray instructions. |
BaiXilin
pushed a commit
to BaiXilin/llvm-fix-vnni-instr-types
that referenced
this pull request
Jan 12, 2025
Also add `tryGetLog2` helper that encapsulates the common pattern:
```
if (takeLog2(..., /*DoFold=*/false)) {
Value * Log2 = takeLog2(..., /*DoFold=*/true);
...
}
```
Closes llvm#122498
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
takeLog2to InstructionCombiner; NFCtryGetLog2; NFCSome cleanup for the purpose of: #121386 (review)