Skip to content

[InstCombine] Simplify add nsw/nuw i1 to or disjoint i1 #118221

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pedroclobo
Copy link
Member

Optimize add nsw i1/add nuw i1 to or disjoint i1.

Alive2: nsw, nuw

Closes #118164.

@llvmbot
Copy link
Member

llvmbot commented Dec 1, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Pedro Lobo (pedroclobo)

Changes

Optimize add nsw i1/add nuw i1 to or disjoint i1.

Alive2: nsw, nuw

Closes #118164.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp (+4-1)
  • (modified) llvm/test/Transforms/InstCombine/add2.ll (+18)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 63725e4ca8113e..1323958a2b4242 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1549,8 +1549,11 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
                                               I.hasNoUnsignedWrap()))
     return R;
   Type *Ty = I.getType();
-  if (Ty->isIntOrIntVectorTy(1))
+  if (Ty->isIntOrIntVectorTy(1)) {
+    if (I.hasNoUnsignedWrap() || I.hasNoSignedWrap())
+      return BinaryOperator::CreateDisjoint(Instruction::Or, LHS, RHS);
     return BinaryOperator::CreateXor(LHS, RHS);
+  }
 
   // X + X --> X << 1
   if (LHS == RHS) {
diff --git a/llvm/test/Transforms/InstCombine/add2.ll b/llvm/test/Transforms/InstCombine/add2.ll
index ae80ab2e92ad15..7107a8443a2edd 100644
--- a/llvm/test/Transforms/InstCombine/add2.ll
+++ b/llvm/test/Transforms/InstCombine/add2.ll
@@ -498,3 +498,21 @@ define i32 @sub_undemanded_low_bits(i32 %x) {
   %shr = lshr i32 %sub, 4
   ret i32 %shr
 }
+
+define i1 @add_nuw_i1(i1 %x, i1 %y) {
+; CHECK-LABEL: @add_nuw_i1(
+; CHECK-NEXT:    [[Z:%.*]] = or disjoint i1 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i1 [[Z]]
+;
+  %z = add nuw i1 %x, %y
+  ret i1 %z
+}
+
+define i1 @add_nsw_i1(i1 %x, i1 %y) {
+; CHECK-LABEL: @add_nsw_i1(
+; CHECK-NEXT:    [[Z:%.*]] = or disjoint i1 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i1 [[Z]]
+;
+  %z = add nsw i1 %x, %y
+  ret i1 %z
+}

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some tests to demonstrate that this change will improve the final codegen/analysis results.

@pedroclobo
Copy link
Member Author

Hey @dtcxzyw. This is one of my first contributions to LLVM. What kind of tests do you mean by "tests to demonstrate that this change will improve the final codegen/analysis results"?

@dtcxzyw
Copy link
Member

dtcxzyw commented Dec 4, 2024

Hey @dtcxzyw. This is one of my first contributions to LLVM. What kind of tests do you mean by "tests to demonstrate that this change will improve the final codegen/analysis results"?

Please refer to https://llvm.org/docs/InstCombineContributorGuide.html#real-world-usefulness. Since this pattern is rare, you should add some tests to show that this patch is important.

cc @scottmcm Does this patch improve Rust codegen?

@scottmcm
Copy link

scottmcm commented Dec 6, 2024

I noticed this looking at a bug where ||ing two bits didn't optimize well, so tried this in godbolt thinking that I could use the add_unchecked intrinsic to do it, but then ended up fixing it in a way that didn't make as nice in IR, but worked well enough for the one specific case I was looking at.

Just figured that since this operation is normalized already, it should normalize to something better in this case, so that -- like normal with or disjoint -- it can be done with addition on platforms where that combines better than xor does.

(Specifically, I was looking at combining carry flags from two uadd.with.overflows, where seeing it xored together is weird, and probably not something that can turn into ADC on x86.)

@dtcxzyw
Copy link
Member

dtcxzyw commented Dec 6, 2024

Specifically, I was looking at combining carry flags from two uadd.with.overflows, where seeing it xored together is weird, and probably not something that can turn into ADC on x86

Can you provide a test case to show that the frontend generates add nsw/nuw i1?

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.

add nuw i1 should simplify to or disjoint i1 (not to xor i1)
4 participants