Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[InstCombine] Add (A & B) - (A & ~B) --> B - (A ^ B) #79717

Closed
wants to merge 2 commits into from

Conversation

AtariDreams
Copy link
Contributor

@AtariDreams AtariDreams commented Jan 28, 2024

(A & B) - (A & ~B) --> B - (A ^ B), and (A & ~B) - (A & B) --> (A ^ B) - B

Alive2 Proof:
https://alive2.llvm.org/ce/z/DWVWYy

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 28, 2024

@llvm/pr-subscribers-llvm-transforms

Author: AtariDreams (AtariDreams)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (+18)
  • (modified) llvm/test/Transforms/InstCombine/sub.ll (+26)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 8a00b75a1f7404..aceb178263811c 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -2371,6 +2371,24 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
       return BinaryOperator::CreateNeg(Y);
   }
 
+  {
+    Value *A, *B;
+    // (A & ~B) - (A & B) => (A ^ B) - B
+    if (match(Op0, m_c_And(m_Value(A), m_Not(m_Value(B)))) &&
+        match(Op1, m_c_And(m_Specific(A), m_Specific(B)))) {
+      return BinaryOperator::CreateSub(Builder.CreateXor(A, B), B);
+    }
+  }
+
+  {
+    Value *A, *B;
+    // (A & B) - (A & ~B) => B - (A ^ B)
+    if (match(Op1, m_c_And(m_Value(A), m_Not(m_Value(B)))) &&
+        match(Op0, m_c_And(m_Specific(A), m_Specific(B)))) {
+      return BinaryOperator::CreateSub(Builder.CreateXor(A, B), B);
+    }
+  }
+
   // (sub (or A, B) (and A, B)) --> (xor A, B)
   {
     Value *A, *B;
diff --git a/llvm/test/Transforms/InstCombine/sub.ll b/llvm/test/Transforms/InstCombine/sub.ll
index 494cdf62c7975e..65142d4e200633 100644
--- a/llvm/test/Transforms/InstCombine/sub.ll
+++ b/llvm/test/Transforms/InstCombine/sub.ll
@@ -2626,3 +2626,29 @@ define i8 @sub_of_adds_2xc(i8 %x, i8 %y) {
   %r = sub i8 %xc, %yc
   ret i8 %r
 }
+
+define i32 @sub_and_xor(i32 %A, i32 %B) {
+; CHECK-LABEL: @sub_and_xor(
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[RES:%.*]] = sub i32 [[TMP1]], [[B]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %xor = xor i32 %B, -1
+  %and1 = and i32 %xor, %A
+  %and2 = and i32 %B, %A
+  %res = sub nsw i32 %and1, %and2
+  ret i32 %res
+}
+
+define i32 @sub_xor_and(i32 %A, i32 %B) {
+; CHECK-LABEL: @sub_xor_and(
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[RES:%.*]] = sub i32 [[TMP1]], [[B]]
+; CHECK-NEXT:    ret i32 [[RES]]
+;
+  %and1 = and i32 %B, %A
+  %xor = xor i32 %B, -1
+  %and2 = and i32 %xor, %A
+  %res = sub nsw i32 %and1, %and2
+  ret i32 %res
+}

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Jan 28, 2024
@nikic
Copy link
Contributor

nikic commented Jan 28, 2024

Did you encounter this pattern in real-world code?

@AtariDreams
Copy link
Contributor Author

Not this in particular, but I've seen code that folds to (A+~B)-(A+B) for example.

@AtariDreams AtariDreams changed the title [InstCombine] Fold (A & B) - (A & ~B) => B - (A ^ B) [InstCombine] Fold (A & B) - (A & ~B) --> B - (A ^ B) Feb 6, 2024
@AtariDreams
Copy link
Contributor Author

_Bytes = _Bytes & ~(_Align - 1); Is a common idiom for rounding up to the nearest alignment.
Difference = Bytes & (_Align - 1) is used to get the modulo.

subtracting these values is useful if you want to know the difference if say, _Bytes were to be the size of a buffer for example.

@topperc
Copy link
Collaborator

topperc commented Feb 8, 2024

@dtcxzyw
Copy link
Member

dtcxzyw commented Feb 9, 2024

Please don't force push to PRs https://llvm.org/docs/GitHub.html#rebasing-pull-requests-and-force-pushes

TBH I always force-push my PRs since I have a bunch of concurrent PRs with different base commits. Without rebasing and force-pushing, ccache works poorly, and I have to waste about an hour every time I switch to another branch and rebuild LLVM :(

@dtcxzyw
Copy link
Member

dtcxzyw commented Feb 9, 2024

_Bytes = _Bytes & ~(_Align - 1); Is a common idiom for rounding up to the nearest alignment.

Should be (_Bytes + _Align - 1) & ~(_Align - 1).

@AtariDreams AtariDreams changed the title [InstCombine] Fold (A & B) - (A & ~B) --> B - (A ^ B) [InstCombine] Add (A & B) - (A & ~B) --> B - (A ^ B) Feb 12, 2024
@nikic
Copy link
Contributor

nikic commented Feb 13, 2024

_Bytes = _Bytes & ~(_Align - 1); Is a common idiom for rounding up to the nearest alignment. Difference = Bytes & (_Align - 1) is used to get the modulo.

subtracting these values is useful if you want to know the difference if say, _Bytes were to be the size of a buffer for example.

~(_Align - 1) will become -_Align and your pattern will not match. If _Align is constant it will also not match.

Please don't invent hypothetical scenarios where it could apply -- provide positive proof where it does apply.

Besides, this pattern is not undef-safe. Your proof specified noundef, and if we drop it we get this: https://alive2.llvm.org/ce/z/-ajcoF It would be necessary to insert freeze instructions to make it correct.

Given that no real-world motivation was provided, I'm closing this.

@nikic nikic closed this Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants