[InstCombine] Simplify select if it combinated and/or/xor #73362

ParkHanbum · 2023-11-24T20:04:04Z

and/or/xor operations can each be changed to sum of logical
operations including operators other than themselves.

x&y -> (x|y) ^ (x^y)
x|y -> (x&y) | (x^y)
x^y -> (x|y) ^ (x&y)

if left of condition of SelectInst is and/or/xor logical
operation and right is equal to 0, -1, or a constant, and
if TrueVal consist of and/or/xor logical operation then we
can optimize this case.

This patch implements this combination.

Proof: https://alive2.llvm.org/ce/z/WW8iRR

Fixed #71792.

llvmbot · 2023-11-24T20:04:33Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: hanbum (ParkHanbum)

Changes

(X & Y) == 0 ? X | Y : X ^ Y --> X ^ Y
(X | Y) == 0 ? X & Y : X ^ Y --> X ^ Y
(X ^ Y) == 0 ? X | Y : X & Y --> X & Y

certain combinations of bitwise operators can be simplified to returning the value of the True or False clause.

Proofs : https://alive2.llvm.org/ce/z/9jidcw

Fixed llvm#71792.

hello! It's an honor to send a PR to LLVM!
thanks for review first!

I'm sending this to get your opinion on whether it's okay to write a patch this way.

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

1 Files Affected:

(modified) llvm/lib/Analysis/InstructionSimplify.cpp (+67)

diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 7ef2ffd23b95e76..a69de0da8b6d046 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -4425,6 +4425,48 @@ Value *llvm::simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
                                   RecursionLimit);
 }
 
+/// Try simplifying the selection command when condition, trueValue, and
+/// falseValue are combinations of special bitwise operators.
+static Value *simplifySelectBitTestSpec(Value *TrueVal, Value *FalseVal,
+                                        Value *X, Value *Y,
+                                        bool TrueWhenUnset) {
+  // (X & Y) == 0 ? X | Y : X ^ Y   --> X ^ Y
+  // (X & Y) == 0 ? X ^ Y : X | Y   --> X | Y
+  // (X & Y) == 1 ? X | Y : X ^ Y   --> X | Y
+  // (X & Y) == 1 ? X ^ Y : X | Y   --> X ^ Y
+  if (match(TrueVal, m_Or(m_Specific(X), m_Specific(Y))) &&
+      match(FalseVal, m_Xor(m_Specific(X), m_Specific(Y)))) {
+    return TrueWhenUnset ? TrueVal : FalseVal;
+  } else if (match(FalseVal, m_Or(m_Specific(X), m_Specific(Y))) &&
+             match(TrueVal, m_Xor(m_Specific(X), m_Specific(Y)))) {
+    return TrueWhenUnset ? TrueVal : FalseVal;
+  }
+  // (X | Y) == 0 ? X & Y : X ^ Y   --> X ^ Y
+  // (X | Y) == 0 ? X ^ Y : X & Y   --> X & Y
+  // (X | Y) == 1 ? X & Y : X ^ Y   --> X & Y
+  // (X | Y) == 1 ? X ^ Y : X & Y   --> X ^ Y
+  if (match(TrueVal, m_And(m_Specific(X), m_Specific(Y))) &&
+      match(FalseVal, m_Xor(m_Specific(X), m_Specific(Y)))) {
+    return TrueWhenUnset ? TrueVal : FalseVal;
+  } else if (match(FalseVal, m_And(m_Specific(X), m_Specific(Y))) &&
+             match(TrueVal, m_Xor(m_Specific(X), m_Specific(Y)))) {
+    return TrueWhenUnset ? TrueVal : FalseVal;
+  }
+  // (X ^ Y) == 0 ? X | Y : X & Y   --> X & Y
+  // (X ^ Y) == 0 ? X & Y : X | Y   --> X | Y
+  // (X ^ Y) == 1 ? X | Y : X & Y   --> X | Y
+  // (X ^ Y) == 1 ? X & Y : X | Y   --> X & Y
+  if (match(TrueVal, m_Or(m_Specific(X), m_Specific(Y))) &&
+      match(FalseVal, m_Xor(m_Specific(X), m_Specific(Y)))) {
+    return TrueWhenUnset ? TrueVal : FalseVal;
+  } else if (match(FalseVal, m_Or(m_Specific(X), m_Specific(Y))) &&
+             match(TrueVal, m_Xor(m_Specific(X), m_Specific(Y)))) {
+    return TrueWhenUnset ? TrueVal : FalseVal;
+  }
+
+  return nullptr;
+}
+
 /// Try to simplify a select instruction when its condition operand is an
 /// integer comparison where one operand of the compare is a constant.
 static Value *simplifySelectBitTest(Value *TrueVal, Value *FalseVal, Value *X,
@@ -4572,12 +4614,37 @@ static Value *simplifySelectWithICmpCond(Value *CondVal, Value *TrueVal,
                                          unsigned MaxRecurse) {
   ICmpInst::Predicate Pred;
   Value *CmpLHS, *CmpRHS;
+  const APInt *C;
   if (!match(CondVal, m_ICmp(Pred, m_Value(CmpLHS), m_Value(CmpRHS))))
     return nullptr;
 
   if (Value *V = simplifyCmpSelOfMaxMin(CmpLHS, CmpRHS, Pred, TrueVal, FalseVal))
     return V;
 
+  if (ICmpInst::isEquality(Pred) && match(CmpRHS, m_APInt(C)) &&
+      (C->isZero() | C->isOne())) {
+    bool which;
+    Value *X, *Y;
+
+    if (Pred == ICmpInst::ICMP_EQ)
+      which = C->isZero() ? false : true;
+    else
+      which = C->isZero() ? true : false;
+
+    if (match(CmpLHS, m_And(m_Value(X), m_Value(Y)))) {
+      if (Value *V = simplifySelectBitTestSpec(TrueVal, FalseVal, X, Y, which))
+        return V;
+    }
+    if (match(CmpLHS, m_Or(m_Value(X), m_Value(Y)))) {
+      if (Value *V = simplifySelectBitTestSpec(TrueVal, FalseVal, X, Y, which))
+        return V;
+    }
+    if (match(CmpLHS, m_Xor(m_Value(X), m_Value(Y)))) {
+      if (Value *V = simplifySelectBitTestSpec(TrueVal, FalseVal, X, Y, which))
+        return V;
+    }
+  }
+
   // Canonicalize ne to eq predicate.
   if (Pred == ICmpInst::ICMP_NE) {
     Pred = ICmpInst::ICMP_EQ;

llvm/lib/Analysis/InstructionSimplify.cpp

llvm/test/Transforms/InstSimplify/select.ll

llvm/lib/Analysis/InstructionSimplify.cpp

ParkHanbum · 2023-12-01T17:48:27Z

I have mistake while resolve conflict. sorry.

llvm/lib/Analysis/InstructionSimplify.cpp

github-actions · 2023-12-06T19:06:05Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Just a missing matcher that came up in llvm#73362

Just a missing matcher that came up in #73362 Closes #86632

`and/or/xor` operations can each be changed to sum of logical operations including operators other than themselves. `x&y -> (x|y) ^ (x^y)` `x|y -> (x&y) | (x^y)` `x^y -> (x|y) ^ (x&y)` This commit contains to test these cases. Proof: https://alive2.llvm.org/ce/z/WW8iRR

goldsteinn · 2024-03-27T17:49:24Z

LGTM.

ParkHanbum · 2024-03-28T03:33:51Z

@goldsteinn thanks for review. I wish I could take TODOs.

goldsteinn · 2024-03-28T17:44:11Z

@goldsteinn thanks for review. I wish I could take TODOs.

You can, I think the way to do this is actually to handle this case in computeKnownBitsFromCmp.

We currently have:

 else if (match(LHS, m_And(m_V, m_APInt(Mask))) &&
               match(RHS, m_APInt(C))) {
      // For one bits in Mask, we can propagate bits from C to V.
      Known.Zero |= ~*C & *Mask;
      Known.One |= *C & *Mask;
      // assume(V | Mask = C)
    }

But we could (and should) also have:

 else if (match(LHS, m_c_And(m_V, m_Value()) &&
               match(RHS, m_APInt(C))) {
      Known.One |= *C;
   }

Likewise for the or case:

 else if (match(LHS, m_c_Or(m_V, m_Value()) &&
               match(RHS, m_APInt(C))) {
      Known.Zero |= ~*C;
   }

The only case that requires special logic is xor where we can't actually deduce any bits, but we
can deduce non-common bits.

That case I think you could handle here in a future patch.

Let me know if you are going to pursue a patch in computeKnownBitsFromCmp.

Edit: Maybe wait a bit, I looked at this a bit and it creates some bugs related to #87015,
so until that is resolved, not much use.

goldsteinn · 2024-03-28T17:45:04Z

@dtcxzyw can you provide a second review of this?

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

goldsteinn · 2024-03-29T01:35:15Z

@goldsteinn thanks for review. I wish I could take TODOs.

You can, I think the way to do this is actually to handle this case in computeKnownBitsFromCmp.

We currently have:
 else if (match(LHS, m_And(m_V, m_APInt(Mask))) &&
               match(RHS, m_APInt(C))) {
      // For one bits in Mask, we can propagate bits from C to V.
      Known.Zero |= ~*C & *Mask;
      Known.One |= *C & *Mask;
      // assume(V | Mask = C)
    }
But we could (and should) also have:
 else if (match(LHS, m_c_And(m_V, m_Value()) &&
               match(RHS, m_APInt(C))) {
      Known.One |= *C;
   }
Likewise for the or case:
 else if (match(LHS, m_c_Or(m_V, m_Value()) &&
               match(RHS, m_APInt(C))) {
      Known.Zero |= ~*C;
   }
The only case that requires special logic is xor where we can't actually deduce any bits, but we can deduce non-common bits.

That case I think you could handle here in a future patch.

Let me know if you are going to pursue a patch in computeKnownBitsFromCmp.

Edit: Maybe wait a bit, I looked at this a bit and it creates some bugs related to #87015, so until that is resolved, not much use.

Think its a non-issue actually. @ParkHanbum lmk if you want to make a patch for this.

`and/or/xor` operations can each be changed to sum of logical operations including operators other than themselves. `x&y -> (x|y) ^ (x^y)` `x|y -> (x&y) | (x^y)` `x^y -> (x|y) ^ (x&y)` if left of condition of `SelectInst` is `and/or/xor` logical operation and right is equal to `0, -1`, or a `constant`, and if `TrueVal` consist of `and/or/xor` logical operation then we can optimize this case. This patch implements this combination. Proof: https://alive2.llvm.org/ce/z/WW8iRR

dtcxzyw

LGTM. BTW the current implementation is a bit larger than the first version I approved. I am not sure whether it is an over-generalization. So I will leave the final decision to @goldsteinn and @nikic.

goldsteinn · 2024-04-01T19:03:52Z

LGTM

goldsteinn · 2024-04-02T15:19:21Z

@ParkHanbum do you have commit access or do you need me to merge for you?

ParkHanbum · 2024-04-03T00:17:02Z

@goldsteinn I don't remember receiving any commit access. can I ask you why you asking that to me because I'm newbie.

…d simple cases. The original intent of the folds (from llvm#71792) where to handle selects where the condition was a bitwise operator compared with zero. During review of the original PR (llvm#73362), however, we ended up over generalizing at the expense of code complexity, and ironically in a way that didn't actually fix the issue as reported. The goal of this PR is to simplify the code and only handle the compares with zero cases that actually show up in the real world. New code handles three cases: 1) `X & Y == 0` implies `X | Y == X ^ Y` thus: - `X & Y == 0 ? X |/^/+ Y : X |/^/+ Y` -> `X |/^/+ Y` (the false arm) - https://alive2.llvm.org/ce/z/jjcduh 2) `X | Y == 0` implies `X == Y == 0` thus for `Op0` and `Op1` s.t `0 Op0 0 == 0 Op1 0 == 0`: - `X & Y == 0 ? X Op0 Y : X Op1 Y` -> `X Op1 Y` - `X & Y == 0 ? 0 : X Op1 Y` -> `X Op1 Y` - https://alive2.llvm.org/ce/z/RBuFQE 3) `X ^ Y == 0` (`X == Y`) implies `X | Y == X & Y`: - `X ^ Y == 0 ? X | Y : X & Y` -> `X & Y` - `X ^ Y == 0 ? X & Y : X | Y` -> `X | Y` - https://alive2.llvm.org/ce/z/SJskbz

ParkHanbum requested a review from nikic as a code owner November 24, 2023 20:04

llvmbot added the llvm:analysis label Nov 24, 2023

dtcxzyw requested a review from goldsteinn November 26, 2023 07:55

goldsteinn reviewed Nov 26, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Nov 26, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Nov 26, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Nov 26, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Nov 26, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Nov 26, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

goldsteinn changed the title ~~[InstCombine] Simplify select if it combinated and/or/xor~~ [InstSimplify] Simplify select if it combinated and/or/xor Nov 26, 2023

goldsteinn reviewed Nov 26, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

ParkHanbum force-pushed the issue_71792_ branch from 02515c5 to 76af103 Compare November 27, 2023 00:20

llvmbot added the llvm:transforms label Nov 27, 2023

ParkHanbum force-pushed the issue_71792_ branch from 76af103 to c06c901 Compare November 27, 2023 09:35

goldsteinn reviewed Nov 27, 2023

View reviewed changes

llvm/test/Transforms/InstSimplify/select.ll Outdated Show resolved Hide resolved

goldsteinn reviewed Nov 27, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Nov 27, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

ParkHanbum force-pushed the issue_71792_ branch 2 times, most recently from 44486d2 to 6539259 Compare November 28, 2023 12:22

goldsteinn reviewed Nov 28, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

ParkHanbum closed this Dec 1, 2023

ParkHanbum force-pushed the issue_71792_ branch from 6539259 to bc80240 Compare December 1, 2023 17:41

ParkHanbum reopened this Dec 1, 2023

ParkHanbum requested a review from goldsteinn December 1, 2023 18:13

goldsteinn reviewed Dec 6, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

goldsteinn reviewed Dec 6, 2023

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

ParkHanbum force-pushed the issue_71792_ branch from 25e67e1 to f0abaf4 Compare December 6, 2023 19:03

goldsteinn mentioned this pull request Mar 26, 2024

[IR] Add m_c_BitwiseLogic in pattern match; NFC #86632

Closed

goldsteinn added a commit to goldsteinn/llvm-project that referenced this pull request Mar 26, 2024

[IR] Add m_c_BitwiseLogic in pattern match; NFC

f0b651b

Just a missing matcher that came up in llvm#73362

goldsteinn added a commit to goldsteinn/llvm-project that referenced this pull request Mar 26, 2024

[IR] Add m_c_BitwiseLogic in pattern match; NFC

0e5db2f

Just a missing matcher that came up in llvm#73362

goldsteinn added a commit that referenced this pull request Mar 26, 2024

[IR] Add m_c_BitwiseLogic in pattern match; NFC

Loading
Loading status checks…

8edb12f

Just a missing matcher that came up in #73362 Closes #86632

ParkHanbum force-pushed the issue_71792_ branch from b6c0124 to 0855e56 Compare March 27, 2024 04:43

ParkHanbum requested a review from goldsteinn March 27, 2024 04:43

dtcxzyw reviewed Mar 28, 2024

View reviewed changes

ParkHanbum force-pushed the issue_71792_ branch from 0855e56 to b96149d Compare March 31, 2024 19:01

ParkHanbum force-pushed the issue_71792_ branch from b96149d to 3ae57ad Compare April 1, 2024 03:55

ParkHanbum requested a review from dtcxzyw April 1, 2024 04:01

dtcxzyw approved these changes Apr 1, 2024

View reviewed changes

goldsteinn mentioned this pull request Apr 1, 2024

[InstCombine] Fold (icmp eq/ne (xor x, y), C1) even if multiuse #87275

Closed

dtcxzyw merged commit 4ef22fc into llvm:main Apr 3, 2024
4 checks passed

nikic mentioned this pull request Apr 23, 2024

[InstCombine] Miscompilation with poison vectors in src_and_eq_C_xor_OrAndNotC #89500

Closed

ParkHanbum mentioned this pull request May 10, 2024

[InstCombine] Try optimizing with knownbits which determined from Cond #91762

Closed

nikic mentioned this pull request Jan 4, 2025

[InstCombine] Bail out on inner disjoint or in foldSelectICmpEq #121635

Closed

goldsteinn mentioned this pull request Jan 5, 2025

[InstCombine] Modify foldSelectICmpEq to only handle more useful and simple cases. #121672

Closed

dtcxzyw mentioned this pull request Jan 7, 2025

[InstCombine] Fold (select (icmp eq (and x, y), 0), (add/xor x, y), F) #121841

Closed

ParkHanbum deleted the issue_71792_ branch January 28, 2025 17:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[InstCombine] Simplify select if it combinated and/or/xor #73362

[InstCombine] Simplify select if it combinated and/or/xor #73362

ParkHanbum commented Nov 24, 2023 •

edited

Loading

llvmbot commented Nov 24, 2023 •

edited

Loading

ParkHanbum commented Dec 1, 2023

github-actions bot commented Dec 6, 2023 •

edited

Loading

goldsteinn commented Mar 27, 2024

ParkHanbum commented Mar 28, 2024

goldsteinn commented Mar 28, 2024 •

edited

Loading

goldsteinn commented Mar 28, 2024

goldsteinn commented Mar 29, 2024 •

edited

Loading

dtcxzyw left a comment

goldsteinn commented Apr 1, 2024

goldsteinn commented Apr 2, 2024

ParkHanbum commented Apr 3, 2024

[InstCombine] Simplify select if it combinated and/or/xor #73362

[InstCombine] Simplify select if it combinated and/or/xor #73362

Conversation

ParkHanbum commented Nov 24, 2023 • edited Loading

llvmbot commented Nov 24, 2023 • edited Loading

ParkHanbum commented Dec 1, 2023

github-actions bot commented Dec 6, 2023 • edited Loading

goldsteinn commented Mar 27, 2024

ParkHanbum commented Mar 28, 2024

goldsteinn commented Mar 28, 2024 • edited Loading

goldsteinn commented Mar 28, 2024

goldsteinn commented Mar 29, 2024 • edited Loading

dtcxzyw left a comment

Choose a reason for hiding this comment

goldsteinn commented Apr 1, 2024

goldsteinn commented Apr 2, 2024

ParkHanbum commented Apr 3, 2024

ParkHanbum commented Nov 24, 2023 •

edited

Loading

llvmbot commented Nov 24, 2023 •

edited

Loading

github-actions bot commented Dec 6, 2023 •

edited

Loading

goldsteinn commented Mar 28, 2024 •

edited

Loading

goldsteinn commented Mar 29, 2024 •

edited

Loading