[InstCombine] Fix miscompilation caused by #90436

[dtcxzyw]

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

Fixes #91127.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

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

Fixes #91127.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+1)
• (added) llvm/test/Transforms/InstCombine/pr91127.ll (+82)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index c60a290ce72e06..7092fb5e509bb1 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -1510,6 +1510,7 @@ InstCombinerImpl::foldICmpTruncWithTruncOrExt(ICmpInst &Cmp,
       std::swap(X, Y);
       Pred = Cmp.getSwappedPredicate(Pred);
     }
+    YIsSExt = !(NoWrapFlags & TruncInst::NoUnsignedWrap);
   }
   // Try to match icmp (trunc nuw X), (zext Y)
   else if (!Cmp.isSigned() &&
diff --git a/llvm/test/Transforms/InstCombine/pr91127.ll b/llvm/test/Transforms/InstCombine/pr91127.ll
new file mode 100644
index 00000000000000..43d431486d2d1b
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr91127.ll
@@ -0,0 +1,82 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+define i1 @test_eq1(i32 %x, i16 %y) {
+; CHECK-LABEL: define i1 @test_eq1(
+; CHECK-SAME: i32 [[X:%.*]], i16 [[Y:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = sext i16 [[Y]] to i32
+; CHECK-NEXT:    [[COND:%.*]] = icmp eq i32 [[TMP1]], [[X]]
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %conv1 = trunc nsw i32 %x to i8
+  %conv2 = trunc nsw i16 %y to i8
+  %cond = icmp eq i8 %conv1, %conv2
+  ret i1 %cond
+}
+
+; FIXME: It is weird that we generate truncs for test_eq2, but not for test_eq1.
+
+define i1 @test_eq2(i32 %x, i16 %y) {
+; CHECK-LABEL: define i1 @test_eq2(
+; CHECK-SAME: i32 [[X:%.*]], i16 [[Y:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = trunc i32 [[X]] to i16
+; CHECK-NEXT:    [[COND:%.*]] = icmp eq i16 [[TMP1]], [[Y]]
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %conv1 = trunc nsw i32 %x to i8
+  %conv2 = trunc nsw i16 %y to i8
+  %cond = icmp eq i8 %conv2, %conv1
+  ret i1 %cond
+}
+
+define i1 @test_ult(i32 %x, i16 %y) {
+; CHECK-LABEL: define i1 @test_ult(
+; CHECK-SAME: i32 [[X:%.*]], i16 [[Y:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = sext i16 [[Y]] to i32
+; CHECK-NEXT:    [[COND:%.*]] = icmp ugt i32 [[TMP1]], [[X]]
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %conv1 = trunc nsw i32 %x to i8
+  %conv2 = trunc nsw i16 %y to i8
+  %cond = icmp ult i8 %conv1, %conv2
+  ret i1 %cond
+}
+
+define i1 @test_slt(i32 %x, i16 %y) {
+; CHECK-LABEL: define i1 @test_slt(
+; CHECK-SAME: i32 [[X:%.*]], i16 [[Y:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = sext i16 [[Y]] to i32
+; CHECK-NEXT:    [[COND:%.*]] = icmp sgt i32 [[TMP1]], [[X]]
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %conv1 = trunc nsw i32 %x to i8
+  %conv2 = trunc nsw i16 %y to i8
+  %cond = icmp slt i8 %conv1, %conv2
+  ret i1 %cond
+}
+
+define i1 @test_ult_nuw(i32 %x, i16 %y) {
+; CHECK-LABEL: define i1 @test_ult_nuw(
+; CHECK-SAME: i32 [[X:%.*]], i16 [[Y:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = zext i16 [[Y]] to i32
+; CHECK-NEXT:    [[COND:%.*]] = icmp ugt i32 [[TMP1]], [[X]]
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %conv1 = trunc nuw nsw i32 %x to i8
+  %conv2 = trunc nuw nsw i16 %y to i8
+  %cond = icmp ult i8 %conv1, %conv2
+  ret i1 %cond
+}
+
+define i1 @test_slt_nuw(i32 %x, i16 %y) {
+; CHECK-LABEL: define i1 @test_slt_nuw(
+; CHECK-SAME: i32 [[X:%.*]], i16 [[Y:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = zext i16 [[Y]] to i32
+; CHECK-NEXT:    [[COND:%.*]] = icmp sgt i32 [[TMP1]], [[X]]
+; CHECK-NEXT:    ret i1 [[COND]]
+;
+  %conv1 = trunc nuw nsw i32 %x to i8
+  %conv2 = trunc nuw nsw i16 %y to i8
+  %cond = icmp slt i8 %conv1, %conv2
+  ret i1 %cond
+}

[dtcxzyw]

I don't know which should be the canonical form. Any thoughts?

[goldsteinn]

I think sext is typically easier to work with than trunc.

[goldsteinn]

Either way though for the trunc case we should be propagating the nowrap flags: https://alive2.llvm.org/ce/z/gb4dQQ

[nikic]

At least historically our position has been that ext is preferable to trunc. (It's possible that this is not the optimal choice anymore now that we have trunc nuw/nsw...)

[goldsteinn]

I think nsw/nuw makes trunc hypothetically equally easy to work with as sext/zext, but since we already have good support for ext seems straightforward to stick with what we got.

[nikic]

LGTM, but can you add the tests to icmp-of-trunc-ext.ll instead of a separate file?

[nikic]

We could also add nneg in these cases.