Skip to content

Conversation

@spaits
Copy link
Contributor

@spaits spaits commented Nov 3, 2025

…nd is the select's condition

It is a prototype that fixes #163108 .

This patch would fix all the possible cases described in the original issue and cases beyond that (other binary operators).

TODO:

  • Extend testing
  • Fix failing test cases in CI
  • Integrate the function for arithmetic operations

@spaits spaits requested a review from dtcxzyw November 3, 2025 21:38
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

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

You can test this locally with the following command:
git-clang-format --diff origin/main HEAD --extensions cpp -- llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index e45cc7b0f..7f78c1349 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -370,13 +370,11 @@ static Instruction *foldBinOpIntoSelectWhenConditionIsOperand(Instruction &I) {
                               ConstantInt::get(Ty, SelLeft->getValue() ^ 1),
                               ConstantInt::get(Ty, SelRight->getValue() ^ 0));
   case Instruction::Add:
-    return SelectInst::Create(CommonInput,
-                              ConstantInt::get(Ty, SelLeft->getValue() + 1),
-                              SelRight);
+    return SelectInst::Create(
+        CommonInput, ConstantInt::get(Ty, SelLeft->getValue() + 1), SelRight);
   case Instruction::Sub:
-    return SelectInst::Create(CommonInput,
-                              ConstantInt::get(Ty, SelLeft->getValue() - 1),
-                              SelRight);
+    return SelectInst::Create(
+        CommonInput, ConstantInt::get(Ty, SelLeft->getValue() - 1), SelRight);
   case Instruction::Mul:
     return SelectInst::Create(CommonInput,
                               ConstantInt::get(Ty, SelLeft->getValue()),
@@ -2510,7 +2508,6 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
     return SelectInst::Create(Cmp, ConstantInt::getNullValue(Ty), Y);
   }
 
-
   if (Instruction *Res = foldBinOpIntoSelectWhenConditionIsOperand(I))
     return Res;
 
@@ -4167,7 +4164,6 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
   if (Instruction *Res = foldBinOpIntoSelectWhenConditionIsOperand(I))
     return Res;
 
-
   // If the operands have no common bits set:
   // or (mul X, Y), X --> add (mul X, Y), X --> mul X, (Y + 1)
   if (match(&I, m_c_DisjointOr(m_OneUse(m_Mul(m_Value(X), m_Value(Y))),
@@ -5271,7 +5267,6 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
   if (Instruction *Res = foldBinOpIntoSelectWhenConditionIsOperand(I))
     return Res;
 
-
   // Fold (X & M) ^ (Y & ~M) -> (X & M) | (Y & ~M)
   // This it a special case in haveNoCommonBitsSet, but the computeKnownBits
   // calls in there are unnecessary as SimplifyDemandedInstructionBits should

@andjo403
Copy link
Contributor

andjo403 commented Nov 5, 2025

maybe it is possible to handle this cases in

if (Op == SI) {
V = IsTrueArm ? SI->getTrueValue() : SI->getFalseValue();
} else if (match(SI->getCondition(),
m_SpecificICmp(IsTrueArm ? ICmpInst::ICMP_EQ
: ICmpInst::ICMP_NE,
m_Specific(Op), m_Value(V))) &&
isGuaranteedNotToBeUndefOrPoison(V)) {
// Pass
} else {
V = Op;
}
Ops.push_back(V);
}

by adding handling of cast something like

    else if (match(Op, m_ZExt(m_Specific(SI->getCondition())))) {
      V = IsTrueArm ? ConstantInt::get(Op->getType(), 1)
                    : ConstantInt::getNullValue(Op->getType());
    } else if (match(Op, m_SExt(m_Specific(SI->getCondition())))) {
      V = IsTrueArm ? ConstantInt::getAllOnesValue(Op->getType())
                    : ConstantInt::getNullValue(Op->getType());
    }

@spaits
Copy link
Contributor Author

spaits commented Nov 5, 2025

Thanks @andjo403 . It works! What to do next? Should I close this PR and you create another one with your code, since you came up with this elegant soultion. Or should I add the code to this PR and have you as another author?

@andjo403
Copy link
Contributor

andjo403 commented Nov 6, 2025

I created a PR with my solution now #166816

@andjo403
Copy link
Contributor

andjo403 commented Nov 6, 2025

as you mention Integrate the for arithmetic operations with a fast search it seems like foldBinOpIntoSelectOrPhi or FoldOpIntoSelect is only called for add/sub with one constant so maybe that can be something for you to continue with if it is not to compile time heavy.

also in foldBinOpIntoSelectOrPhi FoldOpIntoSelect is only called if the first operand is the select but do not think that is a must now that the second parameter is not always a constant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missed Optimization: fold disjoint bitwise ORs (with select/zext i1) into a single constant select

2 participants