[InstCombine] KnownBits::isNonNegative should recognize `b - a` after `a <= b` #145105

rkirsling · 2025-06-20T21:20:56Z

Alive2: https://alive2.llvm.org/ce/z/an9npN
Fixes #142283.

llvmbot · 2025-06-20T21:21:25Z

@llvm/pr-subscribers-llvm-analysis

Author: Ross Kirsling (rkirsling)

Changes

Fixes #142283.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ValueTracking.cpp (+9-1)
(added) llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll (+53)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 73320b556f825..2025079f07572 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -278,7 +278,15 @@ static bool isKnownNonZero(const Value *V, const APInt &DemandedElts,
 
 bool llvm::isKnownNonNegative(const Value *V, const SimplifyQuery &SQ,
                               unsigned Depth) {
-  return computeKnownBits(V, SQ, Depth).isNonNegative();
+  if (computeKnownBits(V, SQ, Depth).isNonNegative())
+    return true;
+
+  Value *X, *Y;
+  if (match(V, m_NSWSub(m_Value(X), m_Value(Y))))
+    if (std::optional<bool> result = isImpliedByDomCondition(ICmpInst::ICMP_SLE, Y, X, SQ.CxtI, SQ.DL))
+      return *result;
+
+  return false;
 }
 
 bool llvm::isKnownPositive(const Value *V, const SimplifyQuery &SQ,
diff --git a/llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll b/llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll
new file mode 100644
index 0000000000000..279a6de4125ed
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+define void @test_as_arg(i8 %a, i8 %b) {
+; CHECK-LABEL: define void @test_as_arg(
+; CHECK-SAME: i8 [[A:%.*]], i8 [[B:%.*]]) {
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i8 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label %[[COND_END:.*]], label %[[COND_FALSE:.*]]
+; CHECK:       [[COND_FALSE]]:
+; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i8 [[B]], [[A]]
+; CHECK-NEXT:    [[CONV:%.*]] = zext nneg i8 [[SUB]] to i16
+; CHECK-NEXT:    call void @subroutine(i16 [[CONV]])
+; CHECK-NEXT:    br label %[[COND_END]]
+; CHECK:       [[COND_END]]:
+; CHECK-NEXT:    ret void
+;
+  %cmp = icmp sgt i8 %a, %b
+  br i1 %cmp, label %cond.end, label %cond.false
+
+cond.false:
+  %sub = sub nsw i8 %b, %a
+  %conv = sext i8 %sub to i16
+  call void @subroutine(i16 %conv)
+  br label %cond.end
+
+cond.end:
+  ret void
+}
+
+declare void @subroutine(i16)
+
+define i16 @test_as_retval(i8 %a, i8 %b) {
+; CHECK-LABEL: define i16 @test_as_retval(
+; CHECK-SAME: i8 [[A:%.*]], i8 [[B:%.*]]) {
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i8 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label %[[COND_TRUE:.*]], label %[[COND_FALSE:.*]]
+; CHECK:       [[COND_TRUE]]:
+; CHECK-NEXT:    ret i16 0
+; CHECK:       [[COND_FALSE]]:
+; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i8 [[B]], [[A]]
+; CHECK-NEXT:    [[CONV:%.*]] = zext nneg i8 [[SUB]] to i16
+; CHECK-NEXT:    ret i16 [[CONV]]
+;
+  %cmp = icmp sgt i8 %a, %b
+  br i1 %cmp, label %cond.true, label %cond.false
+
+cond.true:
+  ret i16 0
+
+cond.false:
+  %sub = sub nsw i8 %b, %a
+  %conv = sext i8 %sub to i16
+  ret i16 %conv
+}

llvmbot · 2025-06-20T21:21:26Z

@llvm/pr-subscribers-llvm-transforms

Author: Ross Kirsling (rkirsling)

Changes

Fixes #142283.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ValueTracking.cpp (+9-1)
(added) llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll (+53)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 73320b556f825..2025079f07572 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -278,7 +278,15 @@ static bool isKnownNonZero(const Value *V, const APInt &DemandedElts,
 
 bool llvm::isKnownNonNegative(const Value *V, const SimplifyQuery &SQ,
                               unsigned Depth) {
-  return computeKnownBits(V, SQ, Depth).isNonNegative();
+  if (computeKnownBits(V, SQ, Depth).isNonNegative())
+    return true;
+
+  Value *X, *Y;
+  if (match(V, m_NSWSub(m_Value(X), m_Value(Y))))
+    if (std::optional<bool> result = isImpliedByDomCondition(ICmpInst::ICMP_SLE, Y, X, SQ.CxtI, SQ.DL))
+      return *result;
+
+  return false;
 }
 
 bool llvm::isKnownPositive(const Value *V, const SimplifyQuery &SQ,
diff --git a/llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll b/llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll
new file mode 100644
index 0000000000000..279a6de4125ed
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+define void @test_as_arg(i8 %a, i8 %b) {
+; CHECK-LABEL: define void @test_as_arg(
+; CHECK-SAME: i8 [[A:%.*]], i8 [[B:%.*]]) {
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i8 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label %[[COND_END:.*]], label %[[COND_FALSE:.*]]
+; CHECK:       [[COND_FALSE]]:
+; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i8 [[B]], [[A]]
+; CHECK-NEXT:    [[CONV:%.*]] = zext nneg i8 [[SUB]] to i16
+; CHECK-NEXT:    call void @subroutine(i16 [[CONV]])
+; CHECK-NEXT:    br label %[[COND_END]]
+; CHECK:       [[COND_END]]:
+; CHECK-NEXT:    ret void
+;
+  %cmp = icmp sgt i8 %a, %b
+  br i1 %cmp, label %cond.end, label %cond.false
+
+cond.false:
+  %sub = sub nsw i8 %b, %a
+  %conv = sext i8 %sub to i16
+  call void @subroutine(i16 %conv)
+  br label %cond.end
+
+cond.end:
+  ret void
+}
+
+declare void @subroutine(i16)
+
+define i16 @test_as_retval(i8 %a, i8 %b) {
+; CHECK-LABEL: define i16 @test_as_retval(
+; CHECK-SAME: i8 [[A:%.*]], i8 [[B:%.*]]) {
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i8 [[A]], [[B]]
+; CHECK-NEXT:    br i1 [[CMP]], label %[[COND_TRUE:.*]], label %[[COND_FALSE:.*]]
+; CHECK:       [[COND_TRUE]]:
+; CHECK-NEXT:    ret i16 0
+; CHECK:       [[COND_FALSE]]:
+; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i8 [[B]], [[A]]
+; CHECK-NEXT:    [[CONV:%.*]] = zext nneg i8 [[SUB]] to i16
+; CHECK-NEXT:    ret i16 [[CONV]]
+;
+  %cmp = icmp sgt i8 %a, %b
+  br i1 %cmp, label %cond.true, label %cond.false
+
+cond.true:
+  ret i16 0
+
+cond.false:
+  %sub = sub nsw i8 %b, %a
+  %conv = sext i8 %sub to i16
+  ret i16 %conv
+}

github-actions · 2025-06-20T21:24:22Z

✅ With the latest revision this PR passed the C/C++ code formatter.

rkirsling · 2025-06-21T00:31:58Z

Hmm, the bot seems to be reporting an unrelated flaky failure.

Also, mentioning @dtcxzyw since I don't have the ability to edit reviewers.

llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll

nikic

This should probably be directly in computeKnownBits() rather than isKnownNonNegative()?

dtcxzyw · 2025-06-21T16:16:29Z

The net effect looks good. But this patch converts more sexts into zext nneg, resulting in less elimination of cast instructions. This regression can be fixed by widening the whole expression tree with canEvaluateSExtd/EvaluateInDifferentType, as we did for sext:

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

Lines 1500 to 1519 in 463ce01

    
           // Try to extend the entire expression tree to the wide destination type. 
        
           if (shouldChangeType(SrcTy, DestTy) && canEvaluateSExtd(Src, DestTy)) { 
        
             // Okay, we can transform this!  Insert the new expression now. 
        
             LLVM_DEBUG( 
        
                 dbgs() << "ICE: EvaluateInDifferentType converting expression type" 
        
                           " to avoid sign extend: " 
        
                        << Sext << '\n'); 
        
             Value *Res = EvaluateInDifferentType(Src, DestTy, true); 
        
             assert(Res->getType() == DestTy); 
        
             // If the high bits are already filled with sign bit, just replace this 
        
             // cast with the result. 
        
             if (ComputeNumSignBits(Res, &Sext) > DestBitSize - SrcBitSize) 
        
               return replaceInstUsesWith(Sext, Res); 
        
             // We need to emit a shl + ashr to do the sign extend. 
        
             Value *ShAmt = ConstantInt::get(DestTy, DestBitSize-SrcBitSize); 
        
             return BinaryOperator::CreateAShr(Builder.CreateShl(Res, ShAmt, "sext"), 
        
                                               ShAmt); 
        
           }

dtcxzyw · 2025-06-22T08:11:00Z

The net effect looks good. But this patch converts more sexts into zext nneg, resulting in less elimination of cast instructions. This regression can be fixed by widening the whole expression tree with canEvaluateSExtd/EvaluateInDifferentType, as we did for sext:

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

Lines 1500 to 1519 in 463ce01

// Try to extend the entire expression tree to the wide destination type.

if (shouldChangeType(SrcTy, DestTy) && canEvaluateSExtd(Src, DestTy)) {

// Okay, we can transform this! Insert the new expression now.

LLVM_DEBUG(

dbgs() << "ICE: EvaluateInDifferentType converting expression type"

" to avoid sign extend: "

<< Sext << '\n');

Value *Res = EvaluateInDifferentType(Src, DestTy, true);

assert(Res->getType() == DestTy);

// If the high bits are already filled with sign bit, just replace this

// cast with the result.

if (ComputeNumSignBits(Res, &Sext) > DestBitSize - SrcBitSize)

return replaceInstUsesWith(Sext, Res);

// We need to emit a shl + ashr to do the sign extend.

Value *ShAmt = ConstantInt::get(DestTy, DestBitSize-SrcBitSize);

return BinaryOperator::CreateAShr(Builder.CreateShl(Res, ShAmt, "sext"),

ShAmt);

}

Ignore me. It looks like a GVN problem.

dtcxzyw

LG

llvm/lib/Analysis/ValueTracking.cpp

rkirsling · 2025-06-23T06:10:45Z

Requesting that this be merged once everybody's happy, since I don't have commit rights yet. 🙇

llvm/lib/Analysis/ValueTracking.cpp

nikic · 2025-06-23T07:07:16Z

llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll

Can you please add a negative test with missing nsw flag?

rkirsling · 2025-06-23T20:38:42Z

Failures on the Linux bot are again unrelated.

nikic

LGTM

… `a <= b` (llvm#145105) Alive2: https://alive2.llvm.org/ce/z/an9npN Fixes llvm#142283.

Add test case.

e6612fe

rkirsling requested a review from nikic as a code owner June 20, 2025 21:20

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Jun 20, 2025

Address issue.

d80031d

rkirsling force-pushed the is-known-non-negative branch from dba9980 to d80031d Compare June 20, 2025 21:28

rebuild linux

905381c

dtcxzyw reviewed Jun 21, 2025

View reviewed changes

llvm/test/Transforms/InstCombine/sub-after-sle-is-non-negative.ll Show resolved Hide resolved

dtcxzyw mentioned this pull request Jun 21, 2025

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

This was referenced Jun 21, 2025

pre-commit: PR145105 dtcxzyw/llvm-opt-benchmark#2481

Closed

pre-commit: PR145105 dtcxzyw/llvm-opt-benchmark#2482

Closed

nikic reviewed Jun 21, 2025

View reviewed changes

rkirsling added 2 commits June 22, 2025 18:13

Add negative tests.

823ede1

Move logic into computeKnownBits.

b4e8603

rkirsling changed the title ~~[InstCombine] isKnownNonNegative should recognize b - a after a <= b~~ [InstCombine] KnownBits::isNonNegative should recognize b - a after a <= b Jun 23, 2025

Merge branch 'main' into is-known-non-negative

6233654

dtcxzyw approved these changes Jun 23, 2025

View reviewed changes

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

Address feedback

68cb10e

rkirsling force-pushed the is-known-non-negative branch from 214e39e to 68cb10e Compare June 23, 2025 05:15

nikic reviewed Jun 23, 2025

View reviewed changes

rkirsling added 2 commits June 23, 2025 10:45

Address nikic's feedback.

1ecadd1

rebuild linux

90d0f80

nikic approved these changes Jun 24, 2025

View reviewed changes

nikic merged commit 7d2293d into llvm:main Jun 24, 2025
6 of 7 checks passed

rkirsling deleted the is-known-non-negative branch June 24, 2025 16:58

DrSergei pushed a commit to DrSergei/llvm-project that referenced this pull request Jun 24, 2025

[InstCombine] KnownBits::isNonNegative should recognize b - a after…

b513c3f

… `a <= b` (llvm#145105) Alive2: https://alive2.llvm.org/ce/z/an9npN Fixes llvm#142283.

rkirsling mentioned this pull request Jun 25, 2025

Request Commit Access For rkirsling #145788

Closed

anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025

[InstCombine] KnownBits::isNonNegative should recognize b - a after…

27c7f10

… `a <= b` (llvm#145105) Alive2: https://alive2.llvm.org/ce/z/an9npN Fixes llvm#142283.

[InstCombine] KnownBits::isNonNegative should recognize b - a after a <= b #145105

[InstCombine] KnownBits::isNonNegative should recognize b - a after a <= b #145105

Uh oh!

Conversation

rkirsling commented Jun 20, 2025 • edited by dtcxzyw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 20, 2025

Uh oh!

llvmbot commented Jun 20, 2025

Uh oh!

github-actions bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rkirsling commented Jun 21, 2025

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

dtcxzyw commented Jun 21, 2025

Uh oh!

dtcxzyw commented Jun 22, 2025

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rkirsling commented Jun 23, 2025

Uh oh!

Uh oh!

nikic Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

rkirsling Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

rkirsling commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[InstCombine] KnownBits::isNonNegative should recognize `b - a` after `a <= b` #145105

[InstCombine] KnownBits::isNonNegative should recognize `b - a` after `a <= b` #145105

rkirsling commented Jun 20, 2025 •

edited by dtcxzyw

Loading

github-actions bot commented Jun 20, 2025 •

edited

Loading

rkirsling commented Jun 23, 2025 •

edited

Loading