[llvm] goldsteinn/takelog2 nfc (PR #122498)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 10 09:37:35 PST 2025
https://github.com/goldsteinn created https://github.com/llvm/llvm-project/pull/122498
- **[InstCombine] Move `takeLog2` to InstructionCombiner; NFC**
- **[InstCombine] Add convenience helper `tryGetLog2`; NFC**
>From b21edc4e29f08984f08fa3e84a8a2fa63ee3e791 Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Fri, 10 Jan 2025 11:32:09 -0600
Subject: [PATCH 1/2] [InstCombine] Move `takeLog2` to InstructionCombiner; NFC
---
.../Transforms/InstCombine/InstCombiner.h | 6 +
.../InstCombine/InstCombineMulDivRem.cpp | 120 +-----------------
.../InstCombine/InstructionCombining.cpp | 99 +++++++++++++++
3 files changed, 111 insertions(+), 114 deletions(-)
diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
index fa6b60cba15aaf..8a87ee0839b7bb 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
@@ -195,6 +195,12 @@ 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);
+
/// 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..576c3bc585db12 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,18 +528,18 @@ 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,
+ if (takeLog2(Op0, /*Depth*/ 0, /*AssumeNonZero*/ false,
/*DoFold*/ false)) {
- Value *Res = takeLog2(Builder, Op0, /*Depth*/ 0, /*AssumeNonZero*/ false,
+ Value *Res = takeLog2(Op0, /*Depth*/ 0, /*AssumeNonZero*/ false,
/*DoFold*/ true);
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,
+ if (takeLog2(Op1, /*Depth*/ 0, /*AssumeNonZero*/ false,
/*DoFold*/ false)) {
- Value *Res = takeLog2(Builder, Op1, /*Depth*/ 0, /*AssumeNonZero*/ false,
+ Value *Res = takeLog2(Op1, /*Depth*/ 0, /*AssumeNonZero*/ false,
/*DoFold*/ true);
BinaryOperator *Shl = BinaryOperator::CreateShl(Op0, Res);
// We can only propegate nuw flag.
@@ -1407,111 +1404,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,9 +1506,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,
+ if (takeLog2(Op1, /*Depth*/ 0, /*AssumeNonZero*/ true,
/*DoFold*/ false)) {
- Value *Res = takeLog2(Builder, Op1, /*Depth*/ 0,
+ Value *Res = takeLog2(Op1, /*Depth*/ 0,
/*AssumeNonZero*/ true, /*DoFold*/ true);
return replaceInstUsesWith(
I, Builder.CreateLShr(Op0, Res, I.getName(), I.isExact()));
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) {
>From fea0bded1aff289c5094fe10c211980dd780ab4d Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Fri, 10 Jan 2025 11:35:47 -0600
Subject: [PATCH 2/2] [InstCombine] Add convenience helper `tryGetLog2`; NFC
This just encapsulates the common pattern:
```
if (takeLog2(..., /*DoFold=*/false)) {
Value * Log2 = takeLog2(..., /*DoFold=*/true);
...
}
```
---
.../llvm/Transforms/InstCombine/InstCombiner.h | 6 ++++++
.../InstCombine/InstCombineMulDivRem.cpp | 16 +++-------------
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
index 8a87ee0839b7bb..213b1b73bed067 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
@@ -201,6 +201,12 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
// 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 576c3bc585db12..e89a2c9579f422 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -528,19 +528,13 @@ Instruction *InstCombinerImpl::visitMul(BinaryOperator &I) {
// (shl Op1, Log2(Op0))
// if Log2(Op1) folds away ->
// (shl Op0, Log2(Op1))
- if (takeLog2(Op0, /*Depth*/ 0, /*AssumeNonZero*/ false,
- /*DoFold*/ false)) {
- Value *Res = takeLog2(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(Op1, /*Depth*/ 0, /*AssumeNonZero*/ false,
- /*DoFold*/ false)) {
- Value *Res = takeLog2(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);
@@ -1506,13 +1500,9 @@ Instruction *InstCombinerImpl::visitUDiv(BinaryOperator &I) {
}
// Op1 udiv Op2 -> Op1 lshr log2(Op2), if log2() folds away.
- if (takeLog2(Op1, /*Depth*/ 0, /*AssumeNonZero*/ true,
- /*DoFold*/ false)) {
- Value *Res = takeLog2(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;
}
More information about the llvm-commits
mailing list