[InstCombine] Fold max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2 #140526

Charukesh827 · 2025-05-19T10:33:07Z

As suggested generalize to fold max(max(x, c1) binop c2, c3) —> max(x binop c2, c3) if c3>=C1* 2 ^ c2 is done.

define i8 @src(i8 %arg0) {
%1 = call i8 @llvm.umax.i8(i8 %arg0, i8 1)
%2 = shl nuw i8 %1, 1
%3 = call i8 @llvm.umax.i8(i8 %2, i8 16)
ret i8 %3
}

define i8 @tgt(i8 %arg0) {
%1 = shl nuw i8 %arg0, 1
%2 = call i8 @llvm.umax.i8(i8 %1, i8 16)
ret i8 %2
}

Closes #139786.

…ue Missed Optimization: max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2 llvm#139786

…ax(x << c2, c3) when c3 >= c1 * 2 ^ c2 This patch fixes issue llvm#139786 where InstCombine where it Missed Optimization: max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2. Pre-committed test in <commit-hash>. Alive2: https://alive2.llvm.org/ce/z/on2tJE

github-actions · 2025-05-19T10:33:29Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-05-19T10:34:03Z

@llvm/pr-subscribers-llvm-transforms

Author: None (Charukesh827)

Changes

As suggested generalize to fold max(max(x, c1) binop c2, c3) —> max(x binop c2, c3) if c3>=C1* 2 ^ c2 is done.

define i8 @src(i8 %arg0) {
%1 = call i8 @llvm.umax.i8(i8 %arg0, i8 1)
%2 = shl nuw i8 %1, 1
%3 = call i8 @llvm.umax.i8(i8 %2, i8 16)
ret i8 %3
}

define i8 @tgt(i8 %arg0) {
%1 = shl nuw i8 %arg0, 1
%2 = call i8 @llvm.umax.i8(i8 %1, i8 16)
ret i8 %2
}

Full diff: https://github.com/llvm/llvm-project/pull/140526.diff

2 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+83)
(added) llvm/test/Transforms/InstCombine/shift-binop.ll (+27)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 3d35bf753c40e..53dd5f803f97b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1171,6 +1171,84 @@ static Instruction *moveAddAfterMinMax(IntrinsicInst *II,
   return IsSigned ? BinaryOperator::CreateNSWAdd(NewMinMax, Add->getOperand(1))
                   : BinaryOperator::CreateNUWAdd(NewMinMax, Add->getOperand(1));
 }
+
+//Try canonicalize min/max(x << shamt, c<<shamt) into max(x, c) << shamt
+static Instruction *moveShiftAfterMinMax(IntrinsicInst *II, InstCombiner::BuilderTy &Builder) {
+  Intrinsic::ID MinMaxID = II->getIntrinsicID();
+  assert((MinMaxID == Intrinsic::smax || MinMaxID == Intrinsic::smin ||
+    MinMaxID == Intrinsic::umax || MinMaxID == Intrinsic::umin) &&
+   "Expected a min or max intrinsic");
+  
+  Value *Op0 = II->getArgOperand(0), *Op1 = II->getArgOperand(1);
+  Value *InnerMax;
+  const APInt *C;
+  if (!match(Op0, m_OneUse(m_BinOp(m_Value(InnerMax), m_APInt(C)))) || 
+      !match(Op1, m_APInt(C)))
+      return nullptr;
+  
+  auto* BinOpInst = cast<BinaryOperator>(Op0);
+  Instruction::BinaryOps BinOp = BinOpInst->getOpcode();
+  Value *X;
+  InnerMax = BinOpInst->getOperand(0);
+  // std::cout<< InnerMax->dump() <<std::endl;
+  if(!match(InnerMax,m_OneUse(m_Intrinsic<Intrinsic::umax>(m_Value(X),m_APInt(C))))){
+  if(!match(InnerMax,m_OneUse(m_Intrinsic<Intrinsic::smax>(m_Value(X),m_APInt(C))))){
+  if(!match(InnerMax,m_OneUse(m_Intrinsic<Intrinsic::umin>(m_Value(X),m_APInt(C))))){
+  if(!match(InnerMax,m_OneUse(m_Intrinsic<Intrinsic::smin>(m_Value(X),m_APInt(C))))){
+     return nullptr;
+  }}}}
+  
+  auto *InnerMaxInst = cast<IntrinsicInst>(InnerMax);
+
+  bool IsSigned = MinMaxID == Intrinsic::smax || MinMaxID == Intrinsic::smin;
+  if((IsSigned && !BinOpInst->hasNoSignedWrap()) ||
+     (!IsSigned && !BinOpInst->hasNoUnsignedWrap())) 
+     return nullptr;
+
+  // Check if BinOp is a left shift
+  if (BinOp != Instruction::Shl) {
+    return nullptr;
+  }
+
+  APInt C2=llvm::dyn_cast<llvm::ConstantInt>(BinOpInst->getOperand(1))->getValue() ;
+  APInt C3=llvm::dyn_cast<llvm::ConstantInt>(II->getArgOperand(1))->getValue();
+  APInt C1=llvm::dyn_cast<llvm::ConstantInt>(InnerMaxInst->getOperand(1))->getValue();
+
+  // Compute C1 * 2^C2
+  APInt Two = APInt(C2.getBitWidth(), 2);
+  APInt Pow2C2 = Two.shl(C2); // 2^C2
+  APInt C1TimesPow2C2 = C1 * Pow2C2; // C1 * 2^C2
+
+  // Check C3 >= C1 * 2^C2
+  if (C3.ult(C1TimesPow2C2)) {
+    return nullptr;
+  }
+
+  //Create new x binop c2
+  Value *NewBinOp = Builder.CreateBinOp(BinOp, InnerMaxInst->getOperand(0), BinOpInst->getOperand(1) );
+  
+  //Create new min/max intrinsic with new binop and c3
+  
+    if(IsSigned){
+      cast<Instruction>(NewBinOp) -> setHasNoSignedWrap(true);
+      cast<Instruction>(NewBinOp) -> setHasNoUnsignedWrap(false);
+    }else{
+      cast<Instruction>(NewBinOp) -> setHasNoUnsignedWrap(true);
+      cast<Instruction>(NewBinOp) -> setHasNoSignedWrap(false);
+    }
+  
+
+  // Get the intrinsic function for MinMaxID
+  Type *Ty = II->getType();
+  Function *MinMaxFn = Intrinsic::getDeclaration(II->getModule(), MinMaxID, {Ty});
+
+  // Create new min/max intrinsic: MinMaxID(NewBinOp, C3) (not inserted)
+  Value *Args[] = {NewBinOp, Op1};
+  Instruction *NewMax = CallInst::Create(MinMaxFn, Args, "", nullptr);
+
+  return NewMax;
+}
+
 /// Match a sadd_sat or ssub_sat which is using min/max to clamp the value.
 Instruction *InstCombinerImpl::matchSAddSubSat(IntrinsicInst &MinMax1) {
   Type *Ty = MinMax1.getType();
@@ -2035,6 +2113,11 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
     if (Instruction *I = moveAddAfterMinMax(II, Builder))
       return I;
 
+    // minmax(x << shamt , c << shamt) -> minmax(x, c) << shamt
+    if (Instruction *I = moveShiftAfterMinMax(II, Builder))  
+      return I;
+
+
     // minmax (X & NegPow2C, Y & NegPow2C) --> minmax(X, Y) & NegPow2C
     const APInt *RHSC;
     if (match(I0, m_OneUse(m_And(m_Value(X), m_NegatedPower2(RHSC)))) &&
diff --git a/llvm/test/Transforms/InstCombine/shift-binop.ll b/llvm/test/Transforms/InstCombine/shift-binop.ll
new file mode 100644
index 0000000000000..78e9c5ea21181
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/shift-binop.ll
@@ -0,0 +1,27 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
+define i8 @src(i8 %arg0) {
+; CHECK-LABEL: @src(
+; CHECK-NEXT:    [[TMP1:%.*]] = shl nuw i8 [[ARG0:%.*]], 1
+; CHECK-NEXT:    [[TMP2:%.*]] = call i8 @llvm.umax.i8(i8 [[TMP1]], i8 16)
+; CHECK-NEXT:    ret i8 [[TMP2]]
+;
+  %1 = call i8 @llvm.umax.i8(i8 %arg0, i8 1)
+  %2 = shl nuw i8 %1, 1
+  %3 = call i8 @llvm.umax.i8(i8 %2, i8 16)
+  ret i8 %3
+}
+
+define i8 @tgt(i8 %arg0) {
+; CHECK-LABEL: @tgt(
+; CHECK-NEXT:    [[TMP1:%.*]] = shl nuw i8 [[ARG0:%.*]], 1
+; CHECK-NEXT:    [[TMP2:%.*]] = call i8 @llvm.umax.i8(i8 [[TMP1]], i8 16)
+; CHECK-NEXT:    ret i8 [[TMP2]]
+;
+  %1 = shl nuw i8 %arg0, 1
+  %2 = call i8 @llvm.umax.i8(i8 %1, i8 16)
+  ret i8 %2
+}
+
+declare i8 @llvm.umax.i8(i8, i8)

github-actions · 2025-05-19T12:50:04Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index cc28ed8e1..75fac5377 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -2202,11 +2202,11 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
     if (Instruction *I = moveAddAfterMinMax(II, Builder))
       return I;
 
-    // max(max(X,C1) binop C2, C3) -> max(X binop C2, max(C1 binop C2, C3)) -> max(X binop C2, C4)
-    if (Instruction *I = reduceMinMax(II, Builder,DL))  
+    // max(max(X,C1) binop C2, C3) -> max(X binop C2, max(C1 binop C2, C3)) ->
+    // max(X binop C2, C4)
+    if (Instruction *I = reduceMinMax(II, Builder, DL))
       return I;
 
-
     // minmax (X & NegPow2C, Y & NegPow2C) --> minmax(X, Y) & NegPow2C
     const APInt *RHSC;
     if (match(I0, m_OneUse(m_And(m_Value(X), m_NegatedPower2(RHSC)))) &&

dtcxzyw

Please add the alive2 proof into the PR description.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

dtcxzyw · 2025-05-19T13:04:14Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+     return nullptr;
+
+  // Check if BinOp is a left shift
+  if (BinOp != Instruction::Shl) {


IIRC, you are trying to implement solution 2 suggested by me: #139786 (comment)

If it is the case, you should generalize it to handle most of binops (excluding div/rem), then use simplifyBinOp and simplifyBinaryIntrinsic to check if min/max(c1 binop c2, c3) folds to c3.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

llvm/test/Transforms/InstCombine/shift-binop.ll

dtcxzyw · 2025-05-19T13:10:42Z

llvm/test/Transforms/InstCombine/shift-binop.ll

@@ -0,0 +1,27 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py


Can you please add more negative tests/vector tests/multi-use tests, as suggested by https://llvm.org/docs/InstCombineContributorGuide.html#tests?

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

dtcxzyw · 2025-05-19T13:15:02Z

llvm/test/Transforms/InstCombine/shift-binop.ll

+; CHECK-NEXT:    [[TMP2:%.*]] = call i8 @llvm.umax.i8(i8 [[TMP1]], i8 16)
+; CHECK-NEXT:    ret i8 [[TMP2]]
+;
+  %1 = call i8 @llvm.umax.i8(i8 %arg0, i8 1)


Use named values.

llvm/test/Transforms/InstCombine/shift-binop.ll

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

"If it is the case, you should generalize it to handle most of binops (excluding div/rem), then use simplifyBinOp and simplifyBinaryIntrinsic to check if min/max(c1 binop c2, c3) folds to c3."

Charukesh827 · 2025-05-20T08:15:01Z

made most of suggested changes. Only thing is i didn't understand what has to be done for generalizing div, Please help me with that.

about the suggestion you gave:

"Solution1: Canonicalize max(x << shamt, c << shamt) into max(x, c) << shamt: https://alive2.llvm.org/ce/z/mQEDAQ
We already did similar things in moveAddAfterMinMax .

Solution2: Generalize to fold max(max(x, c1) binop c2, c3) —> max(x binop c2, c3)."

solution 1 was already available so i didn't make it. I only concentrated on solution 2

Alive2: https://alive2.llvm.org/ce/z/on2tJE

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

dtcxzyw · 2025-05-24T08:30:45Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

 }
+
+// Try canonicalize max(max(X,C1) binop C2, C3) -> max(X binop C2, C3)
+static Instruction *moveShiftAfterMinMax(IntrinsicInst *II,


The function name should be updated.
In fact, this fold can be decomposed into two steps:

max(max(X,C1) binop C2, C3) -> // Associative laws max(max(X binop C2, C1 binop C2), C3) -> // Commutative laws max(X binop C2, max(C1 binop C2, C3)) -> // Constant fold max(X binop C2, C4)

max(X, C1) binop C2 -> max(X binop C2, C1 binop C2) is not always safe for all binops. You can reuse the helper leftDistributesOverRight.

llvm/test/Transforms/InstCombine/shift-binop.ll

dtcxzyw · 2025-05-24T08:37:10Z

solution 1 was already available so i didn't make it.

c << shamt has been constant folded so that instcombine doesn't catch this pattern. Godbolt: https://godbolt.org/z/rndGjEE31

I only concentrated on solution 2

Yeah. I think it is a better solution.

1)max(X, C1) binop C2 -> max(X binop C2, C1 binop C2) is not always safe for all binops. You can reuse the helper leftDistributesOverRight. 2)The function name should be updated. In fact, this fold can be decomposed into two steps: max(max(X,C1) binop C2, C3) -> // Associative laws max(max(X binop C2, C1 binop C2), C3) -> // Commutative laws max(X binop C2, max(C1 binop C2, C3)) -> // Constant fold max(X binop C2, C4)

Charukesh827 · 2025-05-27T16:54:21Z

I wrote rightDistributesOverLeft function with the knowledge i have on this opt, please let me know is anything to be added or removed.

I considered only Add, Sub, Mul, Shl operation, as i was able to verify only these.

another optimization(i didn't write) dominates this optimization
Alive2 : https://alive2.llvm.org/ce/z/sMAnf_

dtcxzyw · 2025-05-30T13:34:31Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

 }
+
+
+static bool rightDistributesOverLeft(Instruction::BinaryOps ROp, bool HasNUW,


Please add some header comments for this function.

Can you provide alive2 proof for this function ((X LOp Y) ROp Z -> (X ROp Z) LOp (Y ROp Z))?
Reference: #140526 (comment)

dtcxzyw · 2025-05-30T13:37:39Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+///  Associative laws max(max(X binop C2, C1 binop C2), C3) -> // Commutative
+///  laws max(X binop C2, max(C1 binop C2, C3)) -> // Constant fold max(X binop
+///  C2, C4)
+


Suggested change

dtcxzyw · 2025-05-30T13:46:11Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+  case Intrinsic::smin:
+    // Signed min/max distribute over addition if no signed wrap.
+    if (HasNSW && ROp == Instruction::Add)
+      return true;


It doesn't hold for smin/smax: https://alive2.llvm.org/ce/z/XFf_U8

dtcxzyw · 2025-05-30T13:48:39Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+  const APInt *C;
+  if (!match(Op0, m_OneUse(m_BinOp(m_Value(InnerMax), m_APInt(C)))) ||
+      !match(Op1, m_APInt(C)))


Suggested change

const APInt *C;

if (!match(Op0, m_OneUse(m_BinOp(m_Value(InnerMax), m_APInt(C)))) ||

!match(Op1, m_APInt(C)))

Constant *C2, *C3;

if (!match(Op0, m_OneUse(m_BinOp(m_Value(InnerMax), m_ImmConstant(C2)))) ||

!match(Op1, m_ImmConstant(C3)))

dtcxzyw · 2025-05-30T13:53:30Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+  // Get constant values
+  APInt C1 = llvm::dyn_cast<llvm::ConstantInt>(InnerMinMaxInst->getOperand(1))
+                 ->getValue();
+  APInt C2 =
+      llvm::dyn_cast<llvm::ConstantInt>(BinOpInst->getOperand(1))->getValue();
+  APInt C3 =
+      llvm::dyn_cast<llvm::ConstantInt>(II->getArgOperand(1))->getValue();
+
+  // Constant fold: Compute C1 binop C2
+  APInt C1BinOpC2, Two, Pow2C2, C1TimesPow2C2;
+  bool overflow = false;
+  switch (BinOp) {
+  case Instruction::Add:
+    C1BinOpC2 = IsSigned ? C1.sadd_ov(C2, overflow) : C1.uadd_ov(C2, overflow);
+    break;
+  case Instruction::Mul:
+    C1BinOpC2 = IsSigned ? C1.smul_ov(C2, overflow) : C1.umul_ov(C2, overflow);
+    break;
+  case Instruction::Sub:
+    C1BinOpC2 = IsSigned ? C1.ssub_ov(C2, overflow) : C1.usub_ov(C2, overflow);
+    break;
+  case Instruction::Shl:
+    // Compute C1 * 2^C2
+    Two = APInt(C2.getBitWidth(), 2);
+    Pow2C2 = Two.shl(C2);        // 2^C2
+    C1TimesPow2C2 = C1 * Pow2C2; // C1 * 2^C2
+
+    // Check C3 >= C1 * 2^C2
+    if (C3.ult(C1TimesPow2C2)) {
+      return nullptr;
+    } else {
+      C1BinOpC2 = C1.shl(C2);
+    }
+    break;
+  default:
+    return nullptr; // Unsupported binary operation
+  }
+
+  // Constant fold: Compute MinMaxID(C1 binop C2, C3) to get C4
+  APInt C4;
+  switch (MinMaxID) {
+  case Intrinsic::umax:
+    C4 = APIntOps::umax(C1BinOpC2, C3);
+    break;
+  case Intrinsic::umin:
+    C4 = APIntOps::umin(C1BinOpC2, C3);
+    break;
+  case Intrinsic::smax:
+    C4 = APIntOps::smax(C1BinOpC2, C3);
+    break;
+  case Intrinsic::smin:
+    C4 = APIntOps::smin(C1BinOpC2, C3);
+    break;
+  default:
+    return nullptr; // Unsupported intrinsic
+  }


Suggested change

// Get constant values

APInt C1 = llvm::dyn_cast<llvm::ConstantInt>(InnerMinMaxInst->getOperand(1))

->getValue();

APInt C2 =

llvm::dyn_cast<llvm::ConstantInt>(BinOpInst->getOperand(1))->getValue();

APInt C3 =

llvm::dyn_cast<llvm::ConstantInt>(II->getArgOperand(1))->getValue();

// Constant fold: Compute C1 binop C2

APInt C1BinOpC2, Two, Pow2C2, C1TimesPow2C2;

bool overflow = false;

switch (BinOp) {

case Instruction::Add:

C1BinOpC2 = IsSigned ? C1.sadd_ov(C2, overflow) : C1.uadd_ov(C2, overflow);

break;

case Instruction::Mul:

C1BinOpC2 = IsSigned ? C1.smul_ov(C2, overflow) : C1.umul_ov(C2, overflow);

break;

case Instruction::Sub:

C1BinOpC2 = IsSigned ? C1.ssub_ov(C2, overflow) : C1.usub_ov(C2, overflow);

break;

case Instruction::Shl:

// Compute C1 * 2^C2

Two = APInt(C2.getBitWidth(), 2);

Pow2C2 = Two.shl(C2); // 2^C2

C1TimesPow2C2 = C1 * Pow2C2; // C1 * 2^C2

// Check C3 >= C1 * 2^C2

if (C3.ult(C1TimesPow2C2)) {

return nullptr;

} else {

C1BinOpC2 = C1.shl(C2);

}

break;

default:

return nullptr; // Unsupported binary operation

}

// Constant fold: Compute MinMaxID(C1 binop C2, C3) to get C4

APInt C4;

switch (MinMaxID) {

case Intrinsic::umax:

C4 = APIntOps::umax(C1BinOpC2, C3);

break;

case Intrinsic::umin:

C4 = APIntOps::umin(C1BinOpC2, C3);

break;

case Intrinsic::smax:

C4 = APIntOps::smax(C1BinOpC2, C3);

break;

case Intrinsic::smin:

C4 = APIntOps::smin(C1BinOpC2, C3);

break;

default:

return nullptr; // Unsupported intrinsic

}

Constant *C1;

if (!match(InnerMinMaxInst->getRHS(), m_ImmConstant(C1))

return nullptr;

Constant *C1BinOpC2 = ConstantFoldBinaryOpOperands(BinOp, C1, C2, DL);

Constant *C4 = ConstantFoldBinaryIntrinsic(MinMaxID, C1BinOpC2, C3, C3->getType(), nullptr);

Charukesh827 · 2025-06-09T17:28:13Z

Changing the code to fit this below given alive2 proof

Charukesh827 · 2025-06-10T06:41:28Z

If we use this Alive2 proof then our aim "Fold max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2" itself becomes false, Shouldn't we do the Alive2 experiment with only one x and all other constant(like Alive2)?

dtcxzyw · 2025-06-18T16:23:35Z

If we use this Alive2 proof then our aim "Fold max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2" itself becomes false

It still holds for umin/umax. It is enough to address the motivating issue.

Shouldn't we do the Alive2 experiment with only one x and all other constant(like Alive2)?

No. We must ensure the transformation is valid for all possible inputs. In your case, both src and tgt return poison, which is always correct.
See also https://llvm.org/docs/InstCombineContributorGuide.html#use-generic-values-in-proofs

Charukesh827 · 2025-07-23T17:28:01Z

made all suggested changes, the rightDistributesOverLeft follows this alive proof

dtcxzyw · 2025-07-24T17:10:00Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

 }
+
+/// Returns weather the it holds for (X LOp Y) ROp Z -> (X ROp Z) LOp (Y ROp Z)
+


Suggested change

dtcxzyw · 2025-07-24T17:12:05Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+                                     bool HasNSW, Intrinsic::ID LOp) {
+  switch (LOp) {
+  case Intrinsic::umax:
+    if (HasNUW && (ROp == Instruction::AShr || ROp == Instruction::LShr ||


ashr/lshr do not have nuw flags.

dtcxzyw · 2025-07-24T17:15:54Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+                                     bool HasNSW, Intrinsic::ID LOp) {
+  switch (LOp) {
+  case Intrinsic::umax:
+    if (HasNUW && (ROp == Instruction::AShr || ROp == Instruction::LShr ||


lshr/ashr do not have nuw flags.

Can you please add some positive/negative tests for all 13 cases? See also https://llvm.org/docs/InstCombineContributorGuide.html#tests

dtcxzyw · 2025-07-24T17:17:47Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+
+///  Try canonicalize max(max(X,C1) binop C2, C3) -> max(X binop C2, max(C1
+///  binop C2, C3))
+/// -> max(X binop C2, C4)  //


Suggested change

/// -> max(X binop C2, C4) //

/// -> max(X binop C2, C4)

dtcxzyw · 2025-07-24T17:18:40Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+  Value *Op0 = II->getArgOperand(0), *Op1 = II->getArgOperand(1);
+  Value *InnerMax;
+  Constant *C2, *C3;
+  if (!match(Op0, m_OneUse(m_BinOp(m_Value(InnerMax), m_ImmConstant(C2)))) ||


Please add some multi-use tests. See also https://llvm.org/docs/InstCombineContributorGuide.html#add-multi-use-tests.

dtcxzyw · 2025-07-24T17:20:30Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+                                        BinOpInst->getOperand(1));
+
+  // Set overflow flags on new binary operation
+  if (auto *NewBinInst = dyn_cast<Instruction>(NewBinOp)) {


It will assert when the binop is not an OverflowingBinaryOperator (e.g, a shift instruction).

dtcxzyw · 2025-07-24T17:21:02Z

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+    if (IsSigned) {
+      NewBinInst->setHasNoSignedWrap(true);
+      NewBinInst->setHasNoUnsignedWrap(false);
+    } else {
+      NewBinInst->setHasNoUnsignedWrap(true);
+      NewBinInst->setHasNoSignedWrap(false);
+    }


Suggested change

if (IsSigned) {

NewBinInst->setHasNoSignedWrap(true);

NewBinInst->setHasNoUnsignedWrap(false);

} else {

NewBinInst->setHasNoUnsignedWrap(true);

NewBinInst->setHasNoSignedWrap(false);

}

NewBinInst->setHasNoSignedWrap(IsSigned);

NewBinInst->setHasNoUnsignedWrap(!IsSigned);

Charukesh827 added 3 commits May 19, 2025 15:17

Add test for shifting binop in InstCombine transformation for the iss…

7e4e9d2

…ue Missed Optimization: max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2 llvm#139786

removed unwanted header (iostream)

b035dcb

Charukesh827 requested a review from nikic as a code owner May 19, 2025 10:33

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels May 19, 2025

dtcxzyw requested changes May 19, 2025

View reviewed changes

dtcxzyw changed the title ~~fix for Issue #139786 - Missed Optimization: max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2~~ [InstCombine] Fold max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2 May 19, 2025

dtcxzyw reviewed May 19, 2025

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp Show resolved Hide resolved

Charukesh827 added 3 commits May 20, 2025 13:16

added negative test

19f60a9

Made suggested changes but couldn't generalize to div/rem as suggested

e317ee2

"If it is the case, you should generalize it to handle most of binops (excluding div/rem), then use simplifyBinOp and simplifyBinaryIntrinsic to check if min/max(c1 binop c2, c3) folds to c3."

added motivation to shift-binop.ll

6f1d83f

Merge branch 'main' into main

09011b6

dtcxzyw reviewed May 24, 2025

View reviewed changes

Charukesh827 and others added 2 commits May 27, 2025 20:34

Merge branch 'main' into main

8e8506b

Charukesh827 force-pushed the main branch from 14122db to daabf95 Compare May 27, 2025 16:36

updated the test for the changed opt

f92f9ce

dtcxzyw reviewed May 30, 2025

View reviewed changes

made all suggested changes

bd41711

dtcxzyw reviewed Jul 24, 2025

View reviewed changes

		@@ -0,0 +1,27 @@
		; NOTE: Assertions have been autogenerated by utils/update_test_checks.py

		}


		static bool rightDistributesOverLeft(Instruction::BinaryOps ROp, bool HasNUW,

		}

		/// Returns weather the it holds for (X LOp Y) ROp Z -> (X ROp Z) LOp (Y ROp Z)

[InstCombine] Fold max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2 #140526

Are you sure you want to change the base?

[InstCombine] Fold max(max(x, c1) << c2, c3) —> max(x << c2, c3) when c3 >= c1 * 2 ^ c2 #140526

Uh oh!

Conversation

Charukesh827 commented May 19, 2025 • edited by dtcxzyw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 19, 2025

Uh oh!

llvmbot commented May 19, 2025

Uh oh!

github-actions bot commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Charukesh827 commented May 20, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dtcxzyw commented May 24, 2025

Uh oh!

Charukesh827 commented May 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dtcxzyw May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Charukesh827 commented Jun 9, 2025

Uh oh!

Charukesh827 commented Jun 10, 2025

Uh oh!

dtcxzyw commented Jun 18, 2025

Uh oh!

Charukesh827 commented Jul 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Charukesh827 commented May 19, 2025 •

edited by dtcxzyw

Loading

github-actions bot commented May 19, 2025 •

edited

Loading

dtcxzyw May 30, 2025 •

edited

Loading