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] Fold trunc nuw/nsw (x xor y) to i1 to x != y #90408

Merged
merged 5 commits into from
Apr 30, 2024

Conversation

YanWQ-monad
Copy link
Contributor

@YanWQ-monad YanWQ-monad commented Apr 28, 2024

Fold:

define i1 @src(i8 %x, i8 %y) {
  %xor = xor i8 %x, %y
  %r = trunc nuw i8 %xor to i1
  ret i1 %r
}

define i1 @tgt(i8 %x, i8 %y) {
  %r = icmp ne i8 %x, %y
  ret i1 %r
}

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

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 28, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Monad (YanWQ-monad)

Changes

Fold:

define i1 @<!-- -->src(i64 %x, i64 %y) {
entry:
  %xor = xor i64 %x, %y
  %r = trunc nuw i64 %xor to i1
  ret i1 %r
}

define i1 @<!-- -->tgt(i64 %x, i64 %y) {
  %r = icmp ne i64 %x, %y
  ret i1 %r
}

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


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp (+7)
  • (modified) llvm/test/Transforms/InstCombine/trunc.ll (+31)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
index 567b27b4630439..c769c28c90c1a9 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -770,6 +770,13 @@ Instruction *InstCombinerImpl::visitTrunc(TruncInst &Trunc) {
         return new ICmpInst(ICmpInst::Predicate::ICMP_EQ, X, Zero);
       }
     }
+
+    if (Trunc.hasNoUnsignedWrap()) {
+      Value *X, *Y;
+      if (match(Src, m_Xor(m_Value(X), m_Value(Y)))) {
+        return new ICmpInst(ICmpInst::ICMP_NE, X, Y);
+      }
+    }
   }
 
   Value *A, *B;
diff --git a/llvm/test/Transforms/InstCombine/trunc.ll b/llvm/test/Transforms/InstCombine/trunc.ll
index e59b2bea6684c0..d4e4b1866349c0 100644
--- a/llvm/test/Transforms/InstCombine/trunc.ll
+++ b/llvm/test/Transforms/InstCombine/trunc.ll
@@ -1054,3 +1054,34 @@ define i8 @drop_both_trunc(i16 %x, i16 %y) {
   %res = trunc nuw nsw i16 %and2 to i8
   ret i8 %res
 }
+
+define i1 @trunc_xor(i8 %x, i8 %y) {
+; CHECK-LABEL: @trunc_xor(
+; CHECK-NEXT:    [[XOR:%.*]] = xor i8 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = trunc i8 [[XOR]] to i1
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %xor = xor i8 %x, %y
+  %r = trunc i8 %xor to i1
+  ret i1 %r
+}
+
+define i1 @trunc_nuw_xor(i8 %x, i8 %y) {
+; CHECK-LABEL: @trunc_nuw_xor(
+; CHECK-NEXT:    [[R:%.*]] = icmp ne i8 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i1 [[R]]
+;
+  %xor = xor i8 %x, %y
+  %r = trunc nuw i8 %xor to i1
+  ret i1 %r
+}
+
+define <2 x i1> @trunc_nuw_xor_vector(<2 x i8> %x, <2 x i8> %y) {
+; CHECK-LABEL: @trunc_nuw_xor_vector(
+; CHECK-NEXT:    [[R:%.*]] = icmp ne <2 x i8> [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret <2 x i1> [[R]]
+;
+  %xor = xor <2 x i8> %x, %y
+  %r = trunc nuw <2 x i8> %xor to <2 x i1>
+  ret <2 x i1> %r
+}

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Apr 28, 2024
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.

LGTM.

Copy link
Contributor

@vfdff vfdff left a comment

Choose a reason for hiding this comment

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

LGTM

@YanWQ-monad YanWQ-monad changed the title [InstCombine] Fold trunc nuw (x xor y) to i1 to x != y [InstCombine] Fold trunc nuw/nsw (x xor y) to i1 to x != y Apr 29, 2024
@dtcxzyw dtcxzyw merged commit 34c89ef into llvm:main Apr 30, 2024
4 checks passed
@asmok-g
Copy link

asmok-g commented May 6, 2024

Heads-up: we're seeing a probable mis-compile after this commit in a google internal test. The function that changes behavior with and without optimization is: https://github.com/google/arolla/blob/65c8389f271b9dce973801074217afb0e35f850c/arolla/memory/simple_buffer.h#L336.

Still checking if this is some UB in the code or a real mis-compile.

@asmok-g
Copy link

asmok-g commented May 6, 2024

The "probable" mis-compile is only happening on arm.

@alexfh
Copy link
Contributor

alexfh commented May 6, 2024

I suppose, this particular function is only involved in what might be a miscompile, when it's inlined into another function, but disabling the optimization just on it using #pragma clang optimize off/on is enough to fix the test we see failing. The problem is that the whole setup leading to the problem is quite convoluted, and there's no isolated reproducer so far.

We're also actively investigating other possibilities like a preexisting UB in the code.

@alexfh
Copy link
Contributor

alexfh commented May 8, 2024

False alarm, it seems. At least, we found UB in the code, fixing which resolves the problem. Apologies for the noise.

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

6 participants