[InstSimplify] Teach simplifyICmpWithConstant about samesign #125899

artagnon · 2025-02-05T17:57:07Z

No description provided.

llvmbot · 2025-02-05T17:57:43Z

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-analysis

Author: Ramkumar Ramachandra (artagnon)

Changes

We have chosen to change ConstantRange::makeAllowedICmpRegion to respect samesign information, noting that ConstantRange::makeExactICmpRegion should not be modified.

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

4 Files Affected:

(modified) llvm/include/llvm/IR/ConstantRange.h (+4-1)
(modified) llvm/lib/Analysis/InstructionSimplify.cpp (+1-1)
(modified) llvm/lib/IR/ConstantRange.cpp (+50-44)
(modified) llvm/test/Analysis/ValueTracking/constant-ranges.ll (+36)

diff --git a/llvm/include/llvm/IR/ConstantRange.h b/llvm/include/llvm/IR/ConstantRange.h
index d086c25390fd227..a40de0a792ef0aa 100644
--- a/llvm/include/llvm/IR/ConstantRange.h
+++ b/llvm/include/llvm/IR/ConstantRange.h
@@ -32,6 +32,7 @@
 #define LLVM_IR_CONSTANTRANGE_H
 
 #include "llvm/ADT/APInt.h"
+#include "llvm/IR/CmpPredicate.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/Support/Compiler.h"
@@ -99,8 +100,10 @@ class [[nodiscard]] ConstantRange {
   /// answer is not representable as a ConstantRange, the return value will be a
   /// proper superset of the above.
   ///
+  /// Note that we respect samesign information on the icmp.
+  ///
   /// Example: Pred = ult and Other = i8 [2, 5) returns Result = [0, 4)
-  static ConstantRange makeAllowedICmpRegion(CmpInst::Predicate Pred,
+  static ConstantRange makeAllowedICmpRegion(CmpPredicate Pred,
                                              const ConstantRange &Other);
 
   /// Produce the largest range such that all values in the returned range
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 3cbc4107433ef3d..7a5a7a39efb1eb9 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -3012,7 +3012,7 @@ static Value *simplifyICmpWithConstant(CmpPredicate Pred, Value *LHS,
   }
 
   // Rule out tautological comparisons (eg., ult 0 or uge 0).
-  ConstantRange RHS_CR = ConstantRange::makeExactICmpRegion(Pred, *C);
+  ConstantRange RHS_CR = ConstantRange::makeAllowedICmpRegion(Pred, *C);
   if (RHS_CR.isEmptySet())
     return ConstantInt::getFalse(ITy);
   if (RHS_CR.isFullSet())
diff --git a/llvm/lib/IR/ConstantRange.cpp b/llvm/lib/IR/ConstantRange.cpp
index 35664353989929d..e776ec38b27bfe2 100644
--- a/llvm/lib/IR/ConstantRange.cpp
+++ b/llvm/lib/IR/ConstantRange.cpp
@@ -95,54 +95,60 @@ KnownBits ConstantRange::toKnownBits() const {
   return Known;
 }
 
-ConstantRange ConstantRange::makeAllowedICmpRegion(CmpInst::Predicate Pred,
+ConstantRange ConstantRange::makeAllowedICmpRegion(CmpPredicate Pred,
                                                    const ConstantRange &CR) {
   if (CR.isEmptySet())
     return CR;
 
-  uint32_t W = CR.getBitWidth();
-  switch (Pred) {
-  default:
-    llvm_unreachable("Invalid ICmp predicate to makeAllowedICmpRegion()");
-  case CmpInst::ICMP_EQ:
-    return CR;
-  case CmpInst::ICMP_NE:
-    if (CR.isSingleElement())
-      return ConstantRange(CR.getUpper(), CR.getLower());
-    return getFull(W);
-  case CmpInst::ICMP_ULT: {
-    APInt UMax(CR.getUnsignedMax());
-    if (UMax.isMinValue())
-      return getEmpty(W);
-    return ConstantRange(APInt::getMinValue(W), std::move(UMax));
-  }
-  case CmpInst::ICMP_SLT: {
-    APInt SMax(CR.getSignedMax());
-    if (SMax.isMinSignedValue())
-      return getEmpty(W);
-    return ConstantRange(APInt::getSignedMinValue(W), std::move(SMax));
-  }
-  case CmpInst::ICMP_ULE:
-    return getNonEmpty(APInt::getMinValue(W), CR.getUnsignedMax() + 1);
-  case CmpInst::ICMP_SLE:
-    return getNonEmpty(APInt::getSignedMinValue(W), CR.getSignedMax() + 1);
-  case CmpInst::ICMP_UGT: {
-    APInt UMin(CR.getUnsignedMin());
-    if (UMin.isMaxValue())
-      return getEmpty(W);
-    return ConstantRange(std::move(UMin) + 1, APInt::getZero(W));
-  }
-  case CmpInst::ICMP_SGT: {
-    APInt SMin(CR.getSignedMin());
-    if (SMin.isMaxSignedValue())
-      return getEmpty(W);
-    return ConstantRange(std::move(SMin) + 1, APInt::getSignedMinValue(W));
-  }
-  case CmpInst::ICMP_UGE:
-    return getNonEmpty(CR.getUnsignedMin(), APInt::getZero(W));
-  case CmpInst::ICMP_SGE:
-    return getNonEmpty(CR.getSignedMin(), APInt::getSignedMinValue(W));
-  }
+  auto CheckPred = [CR](CmpInst::Predicate P) {
+    uint32_t W = CR.getBitWidth();
+    switch (P) {
+    default:
+      llvm_unreachable("Invalid ICmp predicate to makeAllowedICmpRegion()");
+    case CmpInst::ICMP_EQ:
+      return CR;
+    case CmpInst::ICMP_NE:
+      if (CR.isSingleElement())
+        return ConstantRange(CR.getUpper(), CR.getLower());
+      return getFull(W);
+    case CmpInst::ICMP_ULT: {
+      APInt UMax(CR.getUnsignedMax());
+      if (UMax.isMinValue())
+        return getEmpty(W);
+      return ConstantRange(APInt::getMinValue(W), std::move(UMax));
+    }
+    case CmpInst::ICMP_SLT: {
+      APInt SMax(CR.getSignedMax());
+      if (SMax.isMinSignedValue())
+        return getEmpty(W);
+      return ConstantRange(APInt::getSignedMinValue(W), std::move(SMax));
+    }
+    case CmpInst::ICMP_ULE:
+      return getNonEmpty(APInt::getMinValue(W), CR.getUnsignedMax() + 1);
+    case CmpInst::ICMP_SLE:
+      return getNonEmpty(APInt::getSignedMinValue(W), CR.getSignedMax() + 1);
+    case CmpInst::ICMP_UGT: {
+      APInt UMin(CR.getUnsignedMin());
+      if (UMin.isMaxValue())
+        return getEmpty(W);
+      return ConstantRange(std::move(UMin) + 1, APInt::getZero(W));
+    }
+    case CmpInst::ICMP_SGT: {
+      APInt SMin(CR.getSignedMin());
+      if (SMin.isMaxSignedValue())
+        return getEmpty(W);
+      return ConstantRange(std::move(SMin) + 1, APInt::getSignedMinValue(W));
+    }
+    case CmpInst::ICMP_UGE:
+      return getNonEmpty(CR.getUnsignedMin(), APInt::getZero(W));
+    case CmpInst::ICMP_SGE:
+      return getNonEmpty(CR.getSignedMin(), APInt::getSignedMinValue(W));
+    }
+  };
+  if (Pred.hasSameSign())
+    return CheckPred(Pred).unionWith(
+        CheckPred(ICmpInst::getFlippedSignednessPredicate(Pred)));
+  return CheckPred(Pred);
 }
 
 ConstantRange ConstantRange::makeSatisfyingICmpRegion(CmpInst::Predicate Pred,
diff --git a/llvm/test/Analysis/ValueTracking/constant-ranges.ll b/llvm/test/Analysis/ValueTracking/constant-ranges.ll
index c440cfad889d3b4..2e9731895bff3ce 100644
--- a/llvm/test/Analysis/ValueTracking/constant-ranges.ll
+++ b/llvm/test/Analysis/ValueTracking/constant-ranges.ll
@@ -160,6 +160,15 @@ define i1 @srem_posC_okay0(i8 %x) {
   ret i1 %r
 }
 
+define i1 @srem_posC_okay0_samesign(i8 %x) {
+; CHECK-LABEL: @srem_posC_okay0_samesign(
+; CHECK-NEXT:    ret i1 true
+;
+  %val = srem i8 34, %x
+  %r = icmp samesign ule i8 %val, 34
+  ret i1 %r
+}
+
 define i1 @srem_posC_okay1(i8 %x) {
 ; CHECK-LABEL: @srem_posC_okay1(
 ; CHECK-NEXT:    ret i1 true
@@ -169,6 +178,15 @@ define i1 @srem_posC_okay1(i8 %x) {
   ret i1 %r
 }
 
+define i1 @srem_posC_okay1_samesign(i8 %x) {
+; CHECK-LABEL: @srem_posC_okay1_samesign(
+; CHECK-NEXT:    ret i1 true
+;
+  %val = srem i8 34, %x
+  %r = icmp samesign uge i8 %val, -3
+  ret i1 %r
+}
+
 define i1 @srem_negC_okay0(i8 %x) {
 ; CHECK-LABEL: @srem_negC_okay0(
 ; CHECK-NEXT:    ret i1 true
@@ -178,6 +196,15 @@ define i1 @srem_negC_okay0(i8 %x) {
   ret i1 %r
 }
 
+define i1 @srem_negC_okay0_samesign(i8 %x) {
+; CHECK-LABEL: @srem_negC_okay0_samesign(
+; CHECK-NEXT:    ret i1 true
+;
+  %val = srem i8 -34, %x
+  %r = icmp samesign ule i8 %val, 0
+  ret i1 %r
+}
+
 define i1 @srem_negC_okay1(i8 %x) {
 ; CHECK-LABEL: @srem_negC_okay1(
 ; CHECK-NEXT:    ret i1 true
@@ -187,6 +214,15 @@ define i1 @srem_negC_okay1(i8 %x) {
   ret i1 %r
 }
 
+define i1 @srem_negC_okay1_samesign(i8 %x) {
+; CHECK-LABEL: @srem_negC_okay1_samesign(
+; CHECK-NEXT:    ret i1 true
+;
+  %val = srem i8 -34, %x
+  %r = icmp samesign uge i8 %val, -34
+  ret i1 %r
+}
+
 define i1 @srem_posC_fail0(i8 %x) {
 ; CHECK-LABEL: @srem_posC_fail0(
 ; CHECK-NEXT:    [[VAL:%.*]] = srem i8 34, [[X:%.*]]

llvm/lib/Analysis/InstructionSimplify.cpp

llvm/include/llvm/IR/ConstantRange.h

llvm/lib/Analysis/InstructionSimplify.cpp

llvm/lib/IR/ConstantRange.cpp

artagnon · 2025-02-14T16:31:16Z

Patch redone, and ready to review.

llvm/lib/Analysis/InstructionSimplify.cpp

dtcxzyw

LGTM.

nikic

Looks like the corrected version had no hits on llvm-opt-benchmark: dtcxzyw/llvm-opt-benchmark#2130 Why are we making this change?

dtcxzyw · 2025-02-18T09:20:41Z

Looks like the corrected version had no hits on llvm-opt-benchmark: dtcxzyw/llvm-opt-benchmark#2130 Why are we making this change?

It is a compile-time test. Diff: dtcxzyw/llvm-opt-benchmark#2120

artagnon · 2025-02-24T16:17:07Z

Gentle ping @nikic: are you happy with the diff from the correct link?

artagnon · 2025-03-03T11:37:05Z

Gentle ping @nikic.

artagnon · 2025-03-25T10:25:05Z

Gentle ping.

artagnon · 2025-04-03T10:26:59Z

Gentle ping. I don't think the small improvements cost us much, so this patch should be good?

artagnon requested a review from dtcxzyw February 5, 2025 17:57

artagnon requested a review from nikic as a code owner February 5, 2025 17:57

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding labels Feb 5, 2025

dtcxzyw reviewed Feb 6, 2025

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

nikic mentioned this pull request Feb 6, 2025

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

dtcxzyw mentioned this pull request Feb 6, 2025

pre-commit: PR125899 dtcxzyw/llvm-opt-benchmark#2097

Closed

nikic reviewed Feb 6, 2025

View reviewed changes

llvm/include/llvm/IR/ConstantRange.h Outdated Show resolved Hide resolved

llvm/include/llvm/IR/ConstantRange.h Outdated Show resolved Hide resolved

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

artagnon force-pushed the is-icmp-with-constant-samesign branch 2 times, most recently from 17a8bff to f9a2bec Compare February 7, 2025 18:46

dtcxzyw reviewed Feb 10, 2025

View reviewed changes

llvm/lib/IR/ConstantRange.cpp Outdated Show resolved Hide resolved

nikic mentioned this pull request Feb 10, 2025

ConstRange: factor and introduce splitPosNeg (NFC) #126528

Merged

artagnon added 2 commits February 14, 2025 14:30

ValueTracking: pre-commit constrange-samesign tests

d6dc555

InstSimplify: teach simplifyICmpWithConstant about samesign

e86e626

artagnon force-pushed the is-icmp-with-constant-samesign branch from f9a2bec to e86e626 Compare February 14, 2025 16:30

dtcxzyw mentioned this pull request Feb 15, 2025

pre-commit: PR125899 dtcxzyw/llvm-opt-benchmark#2120

Closed

dtcxzyw reviewed Feb 15, 2025

View reviewed changes

llvm/lib/Analysis/InstructionSimplify.cpp Outdated Show resolved Hide resolved

InstSimplify: check UseInstrInfo; address review

bb06f65

This was referenced Feb 18, 2025

Fuzz PR125899 dtcxzyw/llvm-fuzz-service#57

Closed

pre-commit: PR125899 dtcxzyw/llvm-opt-benchmark#2130

Closed

dtcxzyw approved these changes Feb 18, 2025

View reviewed changes

nikic requested changes Feb 18, 2025

View reviewed changes

artagnon changed the title ~~InstSimplify: teach simplifyICmpWithConstant about samesign~~ [InstSimplify] Teach simplifyICmpWithConstant about samesign Feb 24, 2025

artagnon closed this Apr 7, 2025

artagnon deleted the is-icmp-with-constant-samesign branch April 7, 2025 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[InstSimplify] Teach simplifyICmpWithConstant about samesign #125899

[InstSimplify] Teach simplifyICmpWithConstant about samesign #125899

Uh oh!

artagnon commented Feb 5, 2025 •

edited

Loading

Uh oh!

llvmbot commented Feb 5, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

artagnon commented Feb 14, 2025

Uh oh!

Uh oh!

dtcxzyw left a comment

Uh oh!

nikic left a comment

Uh oh!

dtcxzyw commented Feb 18, 2025

Uh oh!

artagnon commented Feb 24, 2025

Uh oh!

artagnon commented Mar 3, 2025

Uh oh!

artagnon commented Mar 25, 2025

Uh oh!

artagnon commented Apr 3, 2025

Uh oh!

Uh oh!

[InstSimplify] Teach simplifyICmpWithConstant about samesign #125899

[InstSimplify] Teach simplifyICmpWithConstant about samesign #125899

Uh oh!

Conversation

artagnon commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

artagnon commented Feb 14, 2025

Uh oh!

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

dtcxzyw commented Feb 18, 2025

Uh oh!

artagnon commented Feb 24, 2025

Uh oh!

artagnon commented Mar 3, 2025

Uh oh!

artagnon commented Mar 25, 2025

Uh oh!

artagnon commented Apr 3, 2025

Uh oh!

Uh oh!

artagnon commented Feb 5, 2025 •

edited

Loading

llvmbot commented Feb 5, 2025 •

edited

Loading