[llvm] [ValueTracking] Respect `samesign` flag in `isKnownInversion` (PR #112390)
Yingwei Zheng via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 15 09:21:55 PDT 2024
https://github.com/dtcxzyw created https://github.com/llvm/llvm-project/pull/112390
In https://github.com/llvm/llvm-project/pull/93591 we introduced `isKnownInversion` and assumes `X` is poison implies `Y` is poison because they share common operands. But after introducing `samesign` this assumption no longer hold if `X` is an icmp has `samesign` flag.
Alive2 link: https://alive2.llvm.org/ce/z/rj3EwQ (Please run it locally with this patch and https://github.com/AliveToolkit/alive2/pull/1098).
This approach is the most conservative way in my mind to address this problem. If `X` has `samesign` flag, it will check if `Y` also has this flag and make sure constant RHS operands have the same sign.
Pros: No API breaking/fast
Cons: less optimization opportunities (But I believe such cases don't exist in real-world code)
I also tried the following solutions:
+ Alternative solution 1: Check `impliesPoison` explicitly and insert `freeze` on demand
+ Pros: instruction-independent
+ Cons: increased compilation time/additional freeze nodes/break API semantic
+ Alternative solution 2: Add an optional `DropFlags` parameter and drop flags in InstCombine
+ Pros: instruction-independent/more optimization opportunities
+ Cons: increased compilation time due to re-enqueue/overhead with small-vector/API breaking
+ Alternative solution 3: Drop all poison-generating flags in InstCombine if `FalseVal` is an ICmp/Instruction
+ Pros: simple/fast
+ Cons: instruction-dependent/hard to extend
Fixes https://github.com/llvm/llvm-project/issues/112350.
>From db2b80d9bac5c80277c6c4c64474d7a9bd5dd548 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Tue, 15 Oct 2024 23:51:56 +0800
Subject: [PATCH 1/2] [InstCombine] Add pre-commit tests. NFC.
---
.../test/Transforms/InstCombine/select-cmp.ll | 84 +++++++++++++++++++
1 file changed, 84 insertions(+)
diff --git a/llvm/test/Transforms/InstCombine/select-cmp.ll b/llvm/test/Transforms/InstCombine/select-cmp.ll
index 234815949d77d4..414284afceebf3 100644
--- a/llvm/test/Transforms/InstCombine/select-cmp.ll
+++ b/llvm/test/Transforms/InstCombine/select-cmp.ll
@@ -480,6 +480,90 @@ define i1 @test_select_inverse_nonconst4(i64 %x, i64 %y, i64 %z, i1 %cond) {
ret i1 %sel
}
+define i1 @test_select_inverse_samesign_true_arm(i64 %x, i64 %y, i1 %cond) {
+; CHECK-LABEL: @test_select_inverse_samesign_true_arm(
+; CHECK-NEXT: [[CMP2:%.*]] = icmp uge i64 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: ret i1 [[SEL]]
+;
+ %cmp1 = icmp samesign ult i64 %x, %y
+ %cmp2 = icmp uge i64 %x, %y
+ %sel = select i1 %cond, i1 %cmp1, i1 %cmp2
+ ret i1 %sel
+}
+
+define i1 @test_select_inverse_samesign_false_arm(i64 %x, i64 %y, i1 %cond) {
+; CHECK-LABEL: @test_select_inverse_samesign_false_arm(
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign uge i64 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: ret i1 [[SEL]]
+;
+ %cmp1 = icmp ult i64 %x, %y
+ %cmp2 = icmp samesign uge i64 %x, %y
+ %sel = select i1 %cond, i1 %cmp1, i1 %cmp2
+ ret i1 %sel
+}
+
+define i1 @test_select_inverse_samesign_both(i64 %x, i64 %y, i1 %cond) {
+; CHECK-LABEL: @test_select_inverse_samesign_both(
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign uge i64 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: ret i1 [[SEL]]
+;
+ %cmp1 = icmp samesign ult i64 %x, %y
+ %cmp2 = icmp samesign uge i64 %x, %y
+ %sel = select i1 %cond, i1 %cmp1, i1 %cmp2
+ ret i1 %sel
+}
+
+define i1 @test_select_inverse_samesign_false_arm_rhsc_same_sign(i64 %x, i64 %y, i1 %cond) {
+; CHECK-LABEL: @test_select_inverse_samesign_false_arm_rhsc_same_sign(
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ugt i64 [[X:%.*]], 10
+; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: ret i1 [[SEL]]
+;
+ %cmp1 = icmp ult i64 %x, 11
+ %cmp2 = icmp samesign ugt i64 %x, 10
+ %sel = select i1 %cond, i1 %cmp1, i1 %cmp2
+ ret i1 %sel
+}
+
+define i1 @test_select_inverse_samesign_true_arm_rhsc_same_sign(i64 %x, i64 %y, i1 %cond) {
+; CHECK-LABEL: @test_select_inverse_samesign_true_arm_rhsc_same_sign(
+; CHECK-NEXT: [[CMP2:%.*]] = icmp ugt i64 [[X:%.*]], 10
+; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: ret i1 [[SEL]]
+;
+ %cmp1 = icmp samesign ult i64 %x, 11
+ %cmp2 = icmp ugt i64 %x, 10
+ %sel = select i1 %cond, i1 %cmp1, i1 %cmp2
+ ret i1 %sel
+}
+
+define i1 @test_select_inverse_samesign_both_rhsc_same_sign(i64 %x, i64 %y, i1 %cond) {
+; CHECK-LABEL: @test_select_inverse_samesign_both_rhsc_same_sign(
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ugt i64 [[X:%.*]], 10
+; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: ret i1 [[SEL]]
+;
+ %cmp1 = icmp samesign ult i64 %x, 11
+ %cmp2 = icmp samesign ugt i64 %x, 10
+ %sel = select i1 %cond, i1 %cmp1, i1 %cmp2
+ ret i1 %sel
+}
+
+define i1 @test_select_inverse_samesign_both_rhsc_diff_sign(i64 %x, i64 %y, i1 %cond) {
+; CHECK-LABEL: @test_select_inverse_samesign_both_rhsc_diff_sign(
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign sgt i64 [[X:%.*]], -1
+; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: ret i1 [[SEL]]
+;
+ %cmp1 = icmp samesign slt i64 %x, 0
+ %cmp2 = icmp samesign sgt i64 %x, -1
+ %sel = select i1 %cond, i1 %cmp1, i1 %cmp2
+ ret i1 %sel
+}
+
define i1 @sel_icmp_two_cmp(i1 %c, i32 %a1, i32 %a2, i32 %a3, i32 %a4) {
; CHECK-LABEL: @sel_icmp_two_cmp(
; CHECK-NEXT: [[CMP1:%.*]] = icmp ule i32 [[A1:%.*]], [[A2:%.*]]
>From 3761ec99d7f79ab51847c82284d2bd124a3e2442 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Tue, 15 Oct 2024 23:59:31 +0800
Subject: [PATCH 2/2] [ValueTracking] Respect `samesign` flag in
`isKnownInversion`
---
llvm/lib/Analysis/ValueTracking.cpp | 14 +++++++++++++-
llvm/test/Transforms/InstCombine/select-cmp.ll | 15 +++++++++------
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index c71d17011d7a0d..a3c4eee7d78a43 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -8526,14 +8526,26 @@ bool llvm::isKnownInversion(const Value *X, const Value *Y) {
!match(Y, m_c_ICmp(Pred2, m_Specific(A), m_Value(C))))
return false;
+ // If they are only differ in predicate. They must both have samesign flag or
+ // not.
if (B == C)
- return Pred1 == ICmpInst::getInversePredicate(Pred2);
+ return Pred1 == ICmpInst::getInversePredicate(Pred2) &&
+ (!cast<ICmpInst>(X)->hasSameSign() ||
+ cast<ICmpInst>(Y)->hasSameSign());
+ ;
// Try to infer the relationship from constant ranges.
const APInt *RHSC1, *RHSC2;
if (!match(B, m_APInt(RHSC1)) || !match(C, m_APInt(RHSC2)))
return false;
+ // They must both have samesign flag or not. And sign bits of two RHSCs should
+ // match.
+ if (cast<ICmpInst>(X)->hasSameSign() &&
+ (!cast<ICmpInst>(Y)->hasSameSign() ||
+ RHSC1->isNonNegative() != RHSC2->isNonNegative()))
+ return false;
+
const auto CR1 = ConstantRange::makeExactICmpRegion(Pred1, *RHSC1);
const auto CR2 = ConstantRange::makeExactICmpRegion(Pred2, *RHSC2);
diff --git a/llvm/test/Transforms/InstCombine/select-cmp.ll b/llvm/test/Transforms/InstCombine/select-cmp.ll
index 414284afceebf3..a2eabf1ede1994 100644
--- a/llvm/test/Transforms/InstCombine/select-cmp.ll
+++ b/llvm/test/Transforms/InstCombine/select-cmp.ll
@@ -494,8 +494,9 @@ define i1 @test_select_inverse_samesign_true_arm(i64 %x, i64 %y, i1 %cond) {
define i1 @test_select_inverse_samesign_false_arm(i64 %x, i64 %y, i1 %cond) {
; CHECK-LABEL: @test_select_inverse_samesign_false_arm(
-; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign uge i64 [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: [[CMP1:%.*]] = icmp ult i64 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign uge i64 [[X]], [[Y]]
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[COND:%.*]], i1 [[CMP1]], i1 [[CMP2]]
; CHECK-NEXT: ret i1 [[SEL]]
;
%cmp1 = icmp ult i64 %x, %y
@@ -518,8 +519,9 @@ define i1 @test_select_inverse_samesign_both(i64 %x, i64 %y, i1 %cond) {
define i1 @test_select_inverse_samesign_false_arm_rhsc_same_sign(i64 %x, i64 %y, i1 %cond) {
; CHECK-LABEL: @test_select_inverse_samesign_false_arm_rhsc_same_sign(
-; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ugt i64 [[X:%.*]], 10
-; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: [[CMP1:%.*]] = icmp ult i64 [[X:%.*]], 11
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ugt i64 [[X]], 10
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[COND:%.*]], i1 [[CMP1]], i1 [[CMP2]]
; CHECK-NEXT: ret i1 [[SEL]]
;
%cmp1 = icmp ult i64 %x, 11
@@ -554,8 +556,9 @@ define i1 @test_select_inverse_samesign_both_rhsc_same_sign(i64 %x, i64 %y, i1 %
define i1 @test_select_inverse_samesign_both_rhsc_diff_sign(i64 %x, i64 %y, i1 %cond) {
; CHECK-LABEL: @test_select_inverse_samesign_both_rhsc_diff_sign(
-; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign sgt i64 [[X:%.*]], -1
-; CHECK-NEXT: [[SEL:%.*]] = xor i1 [[COND:%.*]], [[CMP2]]
+; CHECK-NEXT: [[CMP1:%.*]] = icmp samesign slt i64 [[X:%.*]], 0
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign sgt i64 [[X]], -1
+; CHECK-NEXT: [[SEL:%.*]] = select i1 [[COND:%.*]], i1 [[CMP1]], i1 [[CMP2]]
; CHECK-NEXT: ret i1 [[SEL]]
;
%cmp1 = icmp samesign slt i64 %x, 0
More information about the llvm-commits
mailing list