[CVP] Canonicalize signed minmax into unsigned #82478

dtcxzyw · 2024-02-21T10:17:06Z

This patch turns signed minmax to unsigned to match the behavior for signed icmps.
Alive2: https://alive2.llvm.org/ce/z/UAAM42

llvmbot · 2024-02-21T10:17:36Z

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch turns signed minmax to unsigned to match the behavior for signed icmps.
Alive2: https://alive2.llvm.org/ce/z/UAAM42

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

2 Files Affected:

(modified) llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (+26-6)
(modified) llvm/test/Transforms/CorrelatedValuePropagation/min-max.ll (+70-5)

diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index 9235850de92f3e..6b17d5ff050e14 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -90,6 +90,8 @@ STATISTIC(NumSaturating,
     "Number of saturating arithmetics converted to normal arithmetics");
 STATISTIC(NumNonNull, "Number of function pointer arguments marked non-null");
 STATISTIC(NumMinMax, "Number of llvm.[us]{min,max} intrinsics removed");
+STATISTIC(NumSMinMax,
+          "Number of llvm.s{min,max} intrinsics simplified to unsigned");
 STATISTIC(NumUDivURemsNarrowedExpanded,
           "Number of bound udiv's/urem's expanded");
 STATISTIC(NumZExt, "Number of non-negative deductions");
@@ -528,17 +530,35 @@ static bool processAbsIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
 }
 
 // See if this min/max intrinsic always picks it's one specific operand.
+// If not, check whether we can canonicalize signed minmax into unsigned version
 static bool processMinMaxIntrinsic(MinMaxIntrinsic *MM, LazyValueInfo *LVI) {
   CmpInst::Predicate Pred = CmpInst::getNonStrictPredicate(MM->getPredicate());
   LazyValueInfo::Tristate Result = LVI->getPredicateAt(
       Pred, MM->getLHS(), MM->getRHS(), MM, /*UseBlockValue=*/true);
-  if (Result == LazyValueInfo::Unknown)
-    return false;
+  if (Result != LazyValueInfo::Unknown) {
+    ++NumMinMax;
+    MM->replaceAllUsesWith(MM->getOperand(!Result));
+    MM->eraseFromParent();
+    return true;
+  }
 
-  ++NumMinMax;
-  MM->replaceAllUsesWith(MM->getOperand(!Result));
-  MM->eraseFromParent();
-  return true;
+  if (MM->isSigned() &&
+      ConstantRange::areInsensitiveToSignednessOfICmpPredicate(
+          LVI->getConstantRangeAtUse(MM->getOperandUse(0),
+                                     /*UndefAllowed*/ true),
+          LVI->getConstantRangeAtUse(MM->getOperandUse(1),
+                                     /*UndefAllowed*/ true))) {
+    ++NumSMinMax;
+    IRBuilder<> B(MM);
+    MM->replaceAllUsesWith(B.CreateBinaryIntrinsic(
+        MM->getIntrinsicID() == Intrinsic::smin ? Intrinsic::umin
+                                                : Intrinsic::umax,
+        MM->getLHS(), MM->getRHS()));
+    MM->eraseFromParent();
+    return true;
+  }
+
+  return false;
 }
 
 // Rewrite this with.overflow intrinsic as non-overflowing.
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/min-max.ll b/llvm/test/Transforms/CorrelatedValuePropagation/min-max.ll
index 705b6e96fe9e36..fb14adcf7b61ba 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/min-max.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/min-max.ll
@@ -167,7 +167,7 @@ define i8 @test14(i8 %x) {
 ; CHECK-LABEL: @test14(
 ; CHECK-NEXT:    [[LIM:%.*]] = icmp sge i8 [[X:%.*]], 42
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[LIM]])
-; CHECK-NEXT:    [[R:%.*]] = call i8 @llvm.smin.i8(i8 [[X]], i8 42)
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umin.i8(i8 [[X]], i8 42)
 ; CHECK-NEXT:    ret i8 42
 ;
   %lim = icmp sge i8 %x, 42
@@ -179,8 +179,8 @@ define i8 @test15(i8 %x) {
 ; CHECK-LABEL: @test15(
 ; CHECK-NEXT:    [[LIM:%.*]] = icmp sge i8 [[X:%.*]], 41
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[LIM]])
-; CHECK-NEXT:    [[R:%.*]] = call i8 @llvm.smin.i8(i8 [[X]], i8 42)
-; CHECK-NEXT:    ret i8 [[R]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umin.i8(i8 [[X]], i8 42)
+; CHECK-NEXT:    ret i8 [[TMP1]]
 ;
   %lim = icmp sge i8 %x, 41
   call void @llvm.assume(i1 %lim)
@@ -192,8 +192,8 @@ define i8 @test16(i8 %x) {
 ; CHECK-LABEL: @test16(
 ; CHECK-NEXT:    [[LIM:%.*]] = icmp sge i8 [[X:%.*]], 41
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[LIM]])
-; CHECK-NEXT:    [[R:%.*]] = call i8 @llvm.smax.i8(i8 [[X]], i8 42)
-; CHECK-NEXT:    ret i8 [[R]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umax.i8(i8 [[X]], i8 42)
+; CHECK-NEXT:    ret i8 [[TMP1]]
 ;
   %lim = icmp sge i8 %x, 41
   call void @llvm.assume(i1 %lim)
@@ -235,3 +235,68 @@ define i8 @test19(i8 %x) {
   %r = call i8 @llvm.smax(i8 %x, i8 42)
   ret i8 %r
 }
+
+define i8 @test_smax_to_umax_nneg(i8 %a, i8 %b) {
+; CHECK-LABEL: @test_smax_to_umax_nneg(
+; CHECK-NEXT:    [[NNEG_A:%.*]] = and i8 [[A:%.*]], 127
+; CHECK-NEXT:    [[NNEG_B:%.*]] = and i8 [[B:%.*]], 127
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umax.i8(i8 [[NNEG_A]], i8 [[NNEG_B]])
+; CHECK-NEXT:    ret i8 [[TMP1]]
+;
+  %nneg_a = and i8 %a, 127
+  %nneg_b = and i8 %b, 127
+  %ret = call i8 @llvm.smax.i8(i8 %nneg_a, i8 %nneg_b)
+  ret i8 %ret
+}
+
+define i8 @test_smax_to_umax_neg(i8 %a, i8 %b) {
+; CHECK-LABEL: @test_smax_to_umax_neg(
+; CHECK-NEXT:    [[NEG_A:%.*]] = or i8 [[A:%.*]], -128
+; CHECK-NEXT:    [[NEG_B:%.*]] = or i8 [[B:%.*]], -128
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umax.i8(i8 [[NEG_A]], i8 [[NEG_B]])
+; CHECK-NEXT:    ret i8 [[TMP1]]
+;
+  %neg_a = or i8 %a, 128
+  %neg_b = or i8 %b, 128
+  %ret = call i8 @llvm.smax.i8(i8 %neg_a, i8 %neg_b)
+  ret i8 %ret
+}
+
+define i8 @test_smin_to_umin_nneg(i8 %a, i8 %b) {
+; CHECK-LABEL: @test_smin_to_umin_nneg(
+; CHECK-NEXT:    [[NNEG_A:%.*]] = and i8 [[A:%.*]], 127
+; CHECK-NEXT:    [[NNEG_B:%.*]] = and i8 [[B:%.*]], 127
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umin.i8(i8 [[NNEG_A]], i8 [[NNEG_B]])
+; CHECK-NEXT:    ret i8 [[TMP1]]
+;
+  %nneg_a = and i8 %a, 127
+  %nneg_b = and i8 %b, 127
+  %ret = call i8 @llvm.smin.i8(i8 %nneg_a, i8 %nneg_b)
+  ret i8 %ret
+}
+
+define i8 @test_smin_to_umin_neg(i8 %a, i8 %b) {
+; CHECK-LABEL: @test_smin_to_umin_neg(
+; CHECK-NEXT:    [[NEG_A:%.*]] = or i8 [[A:%.*]], -128
+; CHECK-NEXT:    [[NEG_B:%.*]] = or i8 [[B:%.*]], -128
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umin.i8(i8 [[NEG_A]], i8 [[NEG_B]])
+; CHECK-NEXT:    ret i8 [[TMP1]]
+;
+  %neg_a = or i8 %a, 128
+  %neg_b = or i8 %b, 128
+  %ret = call i8 @llvm.smin.i8(i8 %neg_a, i8 %neg_b)
+  ret i8 %ret
+}
+
+define i8 @test_umax_nneg(i8 %a, i8 %b) {
+; CHECK-LABEL: @test_umax_nneg(
+; CHECK-NEXT:    [[NNEG_A:%.*]] = and i8 [[A:%.*]], 127
+; CHECK-NEXT:    [[NNEG_B:%.*]] = and i8 [[B:%.*]], 127
+; CHECK-NEXT:    [[RET:%.*]] = call i8 @llvm.umax.i8(i8 [[NNEG_A]], i8 [[NNEG_B]])
+; CHECK-NEXT:    ret i8 [[RET]]
+;
+  %nneg_a = and i8 %a, 127
+  %nneg_b = and i8 %b, 127
+  %ret = call i8 @llvm.umax.i8(i8 %nneg_a, i8 %nneg_b)
+  ret i8 %ret
+}

llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp

… in both directions (#82596) This patch uses `getConstantRangeAtUse` in `processMinMaxIntrinsic` to address the comment #82478 (comment). After this patch we can reuse the range result in #82478.

PR Link: llvm/llvm-project#82478

nikic

LGTM

llvm/test/Transforms/CorrelatedValuePropagation/min-max.ll

danilaml · 2024-03-06T14:44:29Z

llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp

@@ -47,11 +47,6 @@ using namespace llvm;

 #define DEBUG_TYPE "correlated-value-propagation"

-static cl::opt<bool> CanonicalizeICmpPredicatesToUnsigned(


Did something change for the example mentioned in https://reviews.llvm.org/D112895#3149487 to warrant removal of this option?

The example is optimized by ConstraintElimination now.

Nice! I expected to find it in the added tests though.

dtcxzyw requested a review from goldsteinn February 21, 2024 10:17

dtcxzyw requested a review from nikic as a code owner February 21, 2024 10:17

llvmbot added the llvm:transforms label Feb 21, 2024

nikic reviewed Feb 21, 2024

View reviewed changes

llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp Outdated Show resolved Hide resolved

dtcxzyw mentioned this pull request Feb 22, 2024

[CVP] Refactor processMinMaxIntrinsic to check non-strict predicate in both directions #82596

Merged

dtcxzyw force-pushed the perf/cvp-canonicalize-signed-minmax branch from fb070a0 to d3411f4 Compare February 22, 2024 13:57

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Feb 22, 2024

pre-commit: test PR82478

66d28b1

PR Link: llvm/llvm-project#82478

dtcxzyw mentioned this pull request Feb 22, 2024

pre-commit: test PR82478 dtcxzyw/llvm-opt-benchmark#257

Closed

dtcxzyw requested a review from nikic February 22, 2024 15:00

nikic approved these changes Feb 22, 2024

View reviewed changes

llvm/test/Transforms/CorrelatedValuePropagation/min-max.ll Outdated Show resolved Hide resolved

dtcxzyw added 2 commits February 23, 2024 00:36

[CVP] Add pre-commit tests. NFC.

87421b6

[CVP] Canonicalize signed minmax into unsigned

2288a86

dtcxzyw force-pushed the perf/cvp-canonicalize-signed-minmax branch from d3411f4 to 2288a86 Compare February 22, 2024 16:44

dtcxzyw merged commit cc83927 into llvm:main Feb 22, 2024
3 of 4 checks passed

dtcxzyw deleted the perf/cvp-canonicalize-signed-minmax branch February 22, 2024 18:42

danilaml reviewed Mar 6, 2024

View reviewed changes

UsmanNadeem mentioned this pull request Mar 18, 2024

ARM USAT instruction not generated after #82478 #85706

Closed

dtcxzyw mentioned this pull request May 31, 2024

[RISCV] Support (truncate (smin (smax X, C1), C2)) for vnclipu in combineTruncToVnclip. #93756

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CVP] Canonicalize signed minmax into unsigned #82478

[CVP] Canonicalize signed minmax into unsigned #82478

dtcxzyw commented Feb 21, 2024

llvmbot commented Feb 21, 2024

nikic left a comment

danilaml Mar 6, 2024

nikic Mar 6, 2024

danilaml Mar 6, 2024

		@@ -47,11 +47,6 @@ using namespace llvm;

		#define DEBUG_TYPE "correlated-value-propagation"

		static cl::opt<bool> CanonicalizeICmpPredicatesToUnsigned(

[CVP] Canonicalize signed minmax into unsigned #82478

[CVP] Canonicalize signed minmax into unsigned #82478

Conversation

dtcxzyw commented Feb 21, 2024

llvmbot commented Feb 21, 2024

nikic left a comment

Choose a reason for hiding this comment

danilaml Mar 6, 2024

Choose a reason for hiding this comment

nikic Mar 6, 2024

Choose a reason for hiding this comment

danilaml Mar 6, 2024

Choose a reason for hiding this comment