Skip to content

Conversation

@fhahn
Copy link
Contributor

@fhahn fhahn commented Oct 23, 2025

We have dedicated decomposition logic for (add %x, -C), but if we have (add nsw %x, -C) we will first apply the generic logic for NSWAdd, which gives worse results for negative constants in practice.

Update the code to first apply the pattern with negative constants.

Helps to remove a number of runtime checks in practice: dtcxzyw/llvm-opt-benchmark#2968

Alive2 proofs for the test changes: https://alive2.llvm.org/ce/z/JfR2Ma

We have dedicated decomposition logic for (add %x, -C), but if we have
(add nsw %x, -C)  we will first apply the generic logic for NSWAdd,
which gives worse results for negative constants in practice.

Update the code to first apply the pattern with negative constants.

Helps to remove a number of runtime checks in practice:
dtcxzyw/llvm-opt-benchmark#2968

Alive2 proofs for the test changes: https://alive2.llvm.org/ce/z/JfR2Ma
@llvmbot
Copy link
Member

llvmbot commented Oct 23, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

We have dedicated decomposition logic for (add %x, -C), but if we have (add nsw %x, -C) we will first apply the generic logic for NSWAdd, which gives worse results for negative constants in practice.

Update the code to first apply the pattern with negative constants.

Helps to remove a number of runtime checks in practice: dtcxzyw/llvm-opt-benchmark#2968

Alive2 proofs for the test changes: https://alive2.llvm.org/ce/z/JfR2Ma


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

3 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+10-10)
  • (modified) llvm/test/Transforms/ConstraintElimination/add-nsw.ll (+2-4)
  • (modified) llvm/test/Transforms/ConstraintElimination/gep-arithmetic-add.ll (+2-4)
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 4acc3f2d84690..d347cedb42988 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -614,6 +614,16 @@ static Decomposition decompose(Value *V,
     return {V, IsKnownNonNegative};
   }
 
+  if (match(V, m_Add(m_Value(Op0), m_ConstantInt(CI))) && CI->isNegative() &&
+      canUseSExt(CI)) {
+    Preconditions.emplace_back(
+        CmpInst::ICMP_UGE, Op0,
+        ConstantInt::get(Op0->getType(), CI->getSExtValue() * -1));
+    if (auto Decomp = MergeResults(Op0, CI, true))
+      return *Decomp;
+    return {V, IsKnownNonNegative};
+  }
+
   if (match(V, m_NSWAdd(m_Value(Op0), m_Value(Op1)))) {
     if (!isKnownNonNegative(Op0, DL))
       Preconditions.emplace_back(CmpInst::ICMP_SGE, Op0,
@@ -627,16 +637,6 @@ static Decomposition decompose(Value *V,
     return {V, IsKnownNonNegative};
   }
 
-  if (match(V, m_Add(m_Value(Op0), m_ConstantInt(CI))) && CI->isNegative() &&
-      canUseSExt(CI)) {
-    Preconditions.emplace_back(
-        CmpInst::ICMP_UGE, Op0,
-        ConstantInt::get(Op0->getType(), CI->getSExtValue() * -1));
-    if (auto Decomp = MergeResults(Op0, CI, true))
-      return *Decomp;
-    return {V, IsKnownNonNegative};
-  }
-
   // Decompose or as an add if there are no common bits between the operands.
   if (match(V, m_DisjointOr(m_Value(Op0), m_ConstantInt(CI)))) {
     if (auto Decomp = MergeResults(Op0, CI, IsSigned))
diff --git a/llvm/test/Transforms/ConstraintElimination/add-nsw.ll b/llvm/test/Transforms/ConstraintElimination/add-nsw.ll
index 5127e92a7c345..4b8ac09801ab2 100644
--- a/llvm/test/Transforms/ConstraintElimination/add-nsw.ll
+++ b/llvm/test/Transforms/ConstraintElimination/add-nsw.ll
@@ -757,8 +757,7 @@ define i1 @add_neg_1_known_sge_ult_1(i32 %a) {
 ; CHECK-NEXT:    [[A_SGE:%.*]] = icmp sge i32 [[A:%.*]], 1
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A_SGE]])
 ; CHECK-NEXT:    [[SUB:%.*]] = add nsw i32 [[A]], -1
-; CHECK-NEXT:    [[C:%.*]] = icmp ult i32 [[SUB]], [[A]]
-; CHECK-NEXT:    ret i1 [[C]]
+; CHECK-NEXT:    ret i1 true
 ;
 entry:
   %a.sge = icmp sge i32 %a, 1
@@ -823,8 +822,7 @@ define i1 @add_neg_3_known_sge_ult_1(i32 %a) {
 ; CHECK-NEXT:    [[A_SGE:%.*]] = icmp sge i32 [[A:%.*]], 3
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A_SGE]])
 ; CHECK-NEXT:    [[SUB:%.*]] = add nsw i32 [[A]], -3
-; CHECK-NEXT:    [[C:%.*]] = icmp ult i32 [[SUB]], [[A]]
-; CHECK-NEXT:    ret i1 [[C]]
+; CHECK-NEXT:    ret i1 true
 ;
 entry:
   %a.sge = icmp sge i32 %a, 3
diff --git a/llvm/test/Transforms/ConstraintElimination/gep-arithmetic-add.ll b/llvm/test/Transforms/ConstraintElimination/gep-arithmetic-add.ll
index 52adc78b4e159..8dcac7832179a 100644
--- a/llvm/test/Transforms/ConstraintElimination/gep-arithmetic-add.ll
+++ b/llvm/test/Transforms/ConstraintElimination/gep-arithmetic-add.ll
@@ -389,8 +389,7 @@ define i1 @gep_count_add_1_sge_known_ult_1(i32 %count, ptr %p) {
 ; CHECK-NEXT:    [[SUB:%.*]] = add nsw i32 [[COUNT]], -1
 ; CHECK-NEXT:    [[SUB_EXT:%.*]] = zext i32 [[SUB]] to i64
 ; CHECK-NEXT:    [[GEP_SUB:%.*]] = getelementptr inbounds i32, ptr [[P]], i64 [[SUB_EXT]]
-; CHECK-NEXT:    [[C:%.*]] = icmp ult ptr [[GEP_SUB]], [[GEP_COUNT]]
-; CHECK-NEXT:    ret i1 [[C]]
+; CHECK-NEXT:    ret i1 true
 ;
 entry:
   %sge = icmp sge i32 %count, 1
@@ -415,8 +414,7 @@ define i1 @gep_count_add_1_sge_known_uge_1(i32 %count, ptr %p) {
 ; CHECK-NEXT:    [[SUB:%.*]] = add nsw i32 [[COUNT]], -1
 ; CHECK-NEXT:    [[SUB_EXT:%.*]] = zext i32 [[SUB]] to i64
 ; CHECK-NEXT:    [[GEP_SUB:%.*]] = getelementptr inbounds i32, ptr [[P]], i64 [[SUB_EXT]]
-; CHECK-NEXT:    [[C:%.*]] = icmp uge ptr [[GEP_SUB]], [[P]]
-; CHECK-NEXT:    ret i1 [[C]]
+; CHECK-NEXT:    ret i1 true
 ;
 entry:
   %sge = icmp sge i32 %count, 1

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fhahn
Copy link
Contributor Author

fhahn commented Oct 23, 2025

cc @AbhayKanhere

@fhahn fhahn merged commit daf0182 into llvm:main Oct 23, 2025
12 checks passed
@fhahn fhahn deleted the ce-reorder-add branch October 23, 2025 11:53
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Oct 23, 2025
…decomp. (#164791)

We have dedicated decomposition logic for (add %x, -C), but if we have
(add nsw %x, -C) we will first apply the generic logic for NSWAdd, which
gives worse results for negative constants in practice.

Update the code to first apply the pattern with negative constants.

Helps to remove a number of runtime checks in practice:
dtcxzyw/llvm-opt-benchmark#2968

Alive2 proofs for the test changes: https://alive2.llvm.org/ce/z/JfR2Ma

PR: llvm/llvm-project#164791
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 23, 2025

LLVM Buildbot has detected a new failure on builder clang-aarch64-quick running on linaro-clang-aarch64-quick while building llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/24418

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'lit :: max-time.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 5
env -u FILECHECK_OPTS "/usr/bin/python3.10" /home/tcwg-buildbot/worker/clang-aarch64-quick/llvm/llvm/utils/lit/lit.py -j1 --order=lexical Inputs/max-time --max-time=5 2>&1  |  FileCheck /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/max-time.py
# executed command: env -u FILECHECK_OPTS /usr/bin/python3.10 /home/tcwg-buildbot/worker/clang-aarch64-quick/llvm/llvm/utils/lit/lit.py -j1 --order=lexical Inputs/max-time --max-time=5
# executed command: FileCheck /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/max-time.py
# .---command stderr------------
# | /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/max-time.py:8:10: error: CHECK: expected string not found in input
# | # CHECK: Skipped: 1
# |          ^
# | <stdin>:3:51: note: scanning from here
# | warning: reached timeout, skipping remaining tests
# |                                                   ^
# | <stdin>:8:2: note: possible intended match here
# |  Skipped: 2 (100.00%)
# |  ^
# | 
# | Input file: <stdin>
# | Check file: /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/max-time.py
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |            1: -- Testing: 2 tests, 1 workers -- 
# |            2: PASS: max-time :: fast.txt (1 of 2) 
# |            3: warning: reached timeout, skipping remaining tests 
# | check:8'0                                                       X error: no match found
# |            4:  
# | check:8'0     ~
# |            5: Testing Time: 5.25s 
# | check:8'0     ~~~~~~~~~~~~~~~~~~~~
# |            6:  
# | check:8'0     ~
# |            7: Total Discovered Tests: 2 
# | check:8'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~
# |            8:  Skipped: 2 (100.00%) 
# | check:8'0     ~~~~~~~~~~~~~~~~~~~~~~
# | check:8'1      ?                     possible intended match
# | >>>>>>
# `-----------------------------
# error: command failed with exit status: 1

--

********************


dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
…vm#164791)

We have dedicated decomposition logic for (add %x, -C), but if we have
(add nsw %x, -C) we will first apply the generic logic for NSWAdd, which
gives worse results for negative constants in practice.

Update the code to first apply the pattern with negative constants.

Helps to remove a number of runtime checks in practice:
dtcxzyw/llvm-opt-benchmark#2968

Alive2 proofs for the test changes: https://alive2.llvm.org/ce/z/JfR2Ma

PR: llvm#164791
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
…vm#164791)

We have dedicated decomposition logic for (add %x, -C), but if we have
(add nsw %x, -C) we will first apply the generic logic for NSWAdd, which
gives worse results for negative constants in practice.

Update the code to first apply the pattern with negative constants.

Helps to remove a number of runtime checks in practice:
dtcxzyw/llvm-opt-benchmark#2968

Alive2 proofs for the test changes: https://alive2.llvm.org/ce/z/JfR2Ma

PR: llvm#164791
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
…vm#164791)

We have dedicated decomposition logic for (add %x, -C), but if we have
(add nsw %x, -C) we will first apply the generic logic for NSWAdd, which
gives worse results for negative constants in practice.

Update the code to first apply the pattern with negative constants.

Helps to remove a number of runtime checks in practice:
dtcxzyw/llvm-opt-benchmark#2968

Alive2 proofs for the test changes: https://alive2.llvm.org/ce/z/JfR2Ma

PR: llvm#164791
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants