[llvm] [Instcombine] Write Instcombine pass to strength reduce lock xadd to lock sub (PR #184715)

Takashi Idobe via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 21 04:27:00 PDT 2026


https://github.com/Takashiidobe updated https://github.com/llvm/llvm-project/pull/184715

>From 0c6e7507bf757c63b7c904231f0e431dbd6c4b92 Mon Sep 17 00:00:00 2001
From: Takashiidobe <idobetakashi at gmail.com>
Date: Tue, 3 Mar 2026 22:34:19 -0500
Subject: [PATCH 1/4] write Instcombine to strength reduce lock xadd to lock
 add

---
 .../InstCombine/InstCombineAtomicRMW.cpp      | 12 +++++
 .../InstCombine/atomicrmw-add-neg.ll          | 49 +++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
index a2e8c695331a6..a3ba3312138bd 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
@@ -14,6 +14,7 @@
 #include "llvm/IR/Instructions.h"
 
 using namespace llvm;
+using namespace llvm::PatternMatch;
 
 /// Return true if and only if the given instruction does not modify the memory
 /// location referenced.  Note that an idemptent atomicrmw may still have
@@ -118,6 +119,17 @@ Instruction *InstCombinerImpl::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
          RMWI.getOrdering() != AtomicOrdering::Unordered &&
          "AtomicRMWs don't make sense with Unordered or NotAtomic");
 
+  // Canonicalize atomicrmw add(ptr, neg(X)) -> atomicrmw sub(ptr, X).
+  // old + (-X) == old - X; the returned old value is identical.
+  // This allows strength reduction on targets where atomic sub is cheaper,
+  // e.g. lock sub instead of lock xadd on x86.
+  Value *X;
+  if (RMWI.getOperation() == AtomicRMWInst::Add &&
+      match(RMWI.getValOperand(), m_Neg(m_Value(X)))) {
+    RMWI.setOperation(AtomicRMWInst::Sub);
+    return replaceOperand(RMWI, 1, X);
+  }
+
   if (!isIdempotentRMW(RMWI))
     return nullptr;
 
diff --git a/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll b/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
new file mode 100644
index 0000000000000..4c1bb20b6a9d4
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
@@ -0,0 +1,49 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -passes=instcombine -S %s | FileCheck %s
+
+; Regression case: atomicrmw sub is unchanged by this transform. 
+define i1 @fn_sub(ptr noundef nonnull align 8 captures(none) dereferenceable(8) %a, i64 noundef %n) {
+; CHECK-LABEL: define i1 @fn_sub(
+; CHECK-SAME: ptr noundef nonnull align 8 captures(none) dereferenceable(8) [[A:%.*]], i64 noundef [[N:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[TMP0:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[TMP0]], [[N]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+entry:
+  %0 = atomicrmw sub ptr %a, i64 %n monotonic, align 8
+  %cmp = icmp eq i64 %0, %n
+  ret i1 %cmp
+}
+
+; Canonicalize atomicrmw add(ptr, neg(n)) -> atomicrmw sub(ptr, n)
+define i1 @fn_add(ptr noundef nonnull align 8 captures(none) dereferenceable(8) %a, i64 noundef %n) {
+; CHECK-LABEL: define i1 @fn_add(
+; CHECK-SAME: ptr noundef nonnull align 8 captures(none) dereferenceable(8) [[A:%.*]], i64 noundef [[N:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[TMP0:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[TMP0]], [[N]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+entry:
+  %sub = sub i64 0, %n
+  %0 = atomicrmw add ptr %a, i64 %sub monotonic, align 8
+  %cmp = icmp eq i64 %0, %n
+  ret i1 %cmp
+}
+
+; nsw neg (signed -n) is also handled; the transform replaces the poison case on neg(INT_MIN) with wrapping sub
+define i1 @fn_adds(ptr noundef nonnull align 8 captures(none) dereferenceable(8) %a, i64 noundef %n) {
+; CHECK-LABEL: define i1 @fn_adds(
+; CHECK-SAME: ptr noundef nonnull align 8 captures(none) dereferenceable(8) [[A:%.*]], i64 noundef [[N:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[OLD:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[OLD]], [[N]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+entry:
+  %neg = sub nsw i64 0, %n
+  %old = atomicrmw add ptr %a, i64 %neg monotonic, align 8
+  %cmp = icmp eq i64 %old, %n
+  ret i1 %cmp
+}

>From 6da6aec78aa7bfe784f26519687f09172715bba3 Mon Sep 17 00:00:00 2001
From: Takashiidobe <idobetakashi at gmail.com>
Date: Thu, 5 Mar 2026 10:45:13 -0500
Subject: [PATCH 2/4] code review feedback: remove unnecessary attributes in
 tests, shrink IR tests, and use Negator::negate instead of handrolling it
 with match

---
 .../InstCombine/InstCombineAtomicRMW.cpp      | 14 +++---
 .../InstCombine/atomicrmw-add-neg.ll          | 50 +++++++------------
 2 files changed, 25 insertions(+), 39 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
index a3ba3312138bd..642a794f97927 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
@@ -14,7 +14,6 @@
 #include "llvm/IR/Instructions.h"
 
 using namespace llvm;
-using namespace llvm::PatternMatch;
 
 /// Return true if and only if the given instruction does not modify the memory
 /// location referenced.  Note that an idemptent atomicrmw may still have
@@ -121,13 +120,12 @@ Instruction *InstCombinerImpl::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
 
   // Canonicalize atomicrmw add(ptr, neg(X)) -> atomicrmw sub(ptr, X).
   // old + (-X) == old - X; the returned old value is identical.
-  // This allows strength reduction on targets where atomic sub is cheaper,
-  // e.g. lock sub instead of lock xadd on x86.
-  Value *X;
-  if (RMWI.getOperation() == AtomicRMWInst::Add &&
-      match(RMWI.getValOperand(), m_Neg(m_Value(X)))) {
-    RMWI.setOperation(AtomicRMWInst::Sub);
-    return replaceOperand(RMWI, 1, X);
+  if (RMWI.getOperation() == AtomicRMWInst::Add) {
+    if (Value *X = Negator::Negate(/*LHSIsZero=*/true, /*IsNSW=*/false,
+                                   RMWI.getValOperand(), *this)) {
+      RMWI.setOperation(AtomicRMWInst::Sub);
+      return replaceOperand(RMWI, 1, X);
+    }
   }
 
   if (!isIdempotentRMW(RMWI))
diff --git a/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll b/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
index 4c1bb20b6a9d4..9010bdb166bfc 100644
--- a/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
+++ b/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
@@ -1,49 +1,37 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
 ; RUN: opt -passes=instcombine -S %s | FileCheck %s
 
-; Regression case: atomicrmw sub is unchanged by this transform. 
-define i1 @fn_sub(ptr noundef nonnull align 8 captures(none) dereferenceable(8) %a, i64 noundef %n) {
-; CHECK-LABEL: define i1 @fn_sub(
-; CHECK-SAME: ptr noundef nonnull align 8 captures(none) dereferenceable(8) [[A:%.*]], i64 noundef [[N:%.*]]) {
-; CHECK-NEXT:  [[ENTRY:.*:]]
+; Regression case: atomicrmw sub is unchanged by this transform.
+define i64 @fn_sub(ptr %a, i64 %n) {
+; CHECK-LABEL: define i64 @fn_sub(
+; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
 ; CHECK-NEXT:    [[TMP0:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[TMP0]], [[N]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i64 [[TMP0]]
 ;
-entry:
-  %0 = atomicrmw sub ptr %a, i64 %n monotonic, align 8
-  %cmp = icmp eq i64 %0, %n
-  ret i1 %cmp
+  %1 = atomicrmw sub ptr %a, i64 %n monotonic
+  ret i64 %1
 }
 
 ; Canonicalize atomicrmw add(ptr, neg(n)) -> atomicrmw sub(ptr, n)
-define i1 @fn_add(ptr noundef nonnull align 8 captures(none) dereferenceable(8) %a, i64 noundef %n) {
-; CHECK-LABEL: define i1 @fn_add(
-; CHECK-SAME: ptr noundef nonnull align 8 captures(none) dereferenceable(8) [[A:%.*]], i64 noundef [[N:%.*]]) {
-; CHECK-NEXT:  [[ENTRY:.*:]]
+define i64 @fn_add(ptr %a, i64 %n) {
+; CHECK-LABEL: define i64 @fn_add(
+; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
 ; CHECK-NEXT:    [[TMP0:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[TMP0]], [[N]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i64 [[TMP0]]
 ;
-entry:
   %sub = sub i64 0, %n
-  %0 = atomicrmw add ptr %a, i64 %sub monotonic, align 8
-  %cmp = icmp eq i64 %0, %n
-  ret i1 %cmp
+  %1 = atomicrmw add ptr %a, i64 %sub monotonic
+  ret i64 %1
 }
 
 ; nsw neg (signed -n) is also handled; the transform replaces the poison case on neg(INT_MIN) with wrapping sub
-define i1 @fn_adds(ptr noundef nonnull align 8 captures(none) dereferenceable(8) %a, i64 noundef %n) {
-; CHECK-LABEL: define i1 @fn_adds(
-; CHECK-SAME: ptr noundef nonnull align 8 captures(none) dereferenceable(8) [[A:%.*]], i64 noundef [[N:%.*]]) {
-; CHECK-NEXT:  [[ENTRY:.*:]]
+define i64 @fn_adds(ptr %a, i64 %n) {
+; CHECK-LABEL: define i64 @fn_adds(
+; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
 ; CHECK-NEXT:    [[OLD:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[OLD]], [[N]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i64 [[OLD]]
 ;
-entry:
   %neg = sub nsw i64 0, %n
-  %old = atomicrmw add ptr %a, i64 %neg monotonic, align 8
-  %cmp = icmp eq i64 %old, %n
-  ret i1 %cmp
+  %old = atomicrmw add ptr %a, i64 %neg monotonic
+  ret i64 %old
 }

>From afeae0e687bb82c5e1e9ded4048df5e4a9402cc7 Mon Sep 17 00:00:00 2001
From: Takashiidobe <idobetakashi at gmail.com>
Date: Thu, 5 Mar 2026 19:51:46 -0500
Subject: [PATCH 3/4] code review feedback + handle case when n is both a
 constant and a variable

---
 .../InstCombine/InstCombineAtomicRMW.cpp      | 20 ++++---
 llvm/test/CodeGen/X86/atomicrmw-add-neg.ll    | 59 +++++++++++++++++++
 .../InstCombine/atomicrmw-add-neg.ll          | 47 +++++++++++++--
 3 files changed, 113 insertions(+), 13 deletions(-)
 create mode 100644 llvm/test/CodeGen/X86/atomicrmw-add-neg.ll

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
index 642a794f97927..2c4b74a2652cc 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
@@ -118,13 +118,19 @@ Instruction *InstCombinerImpl::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
          RMWI.getOrdering() != AtomicOrdering::Unordered &&
          "AtomicRMWs don't make sense with Unordered or NotAtomic");
 
-  // Canonicalize atomicrmw add(ptr, neg(X)) -> atomicrmw sub(ptr, X).
-  // old + (-X) == old - X; the returned old value is identical.
-  if (RMWI.getOperation() == AtomicRMWInst::Add) {
-    if (Value *X = Negator::Negate(/*LHSIsZero=*/true, /*IsNSW=*/false,
-                                   RMWI.getValOperand(), *this)) {
-      RMWI.setOperation(AtomicRMWInst::Sub);
-      return replaceOperand(RMWI, 1, X);
+  // Canonicalize atomicrmw add(ptr, neg(X)) -> atomicrmw sub(ptr, X)
+  //              atomicrmw sub(ptr, neg(X)) -> atomicrmw add(ptr, X)
+  // old + (-X) == old - X and old - (-X) == old + X; the returned old value
+  // is identical in both cases.
+  auto Op = RMWI.getOperation();
+  if (Op == AtomicRMWInst::Add || Op == AtomicRMWInst::Sub) {
+    Value *Val = RMWI.getValOperand();
+    if (!isa<Constant>(Val) || match(Val, PatternMatch::m_Negative())) {
+      if (Value *X = Negator::Negate(true, false, Val, *this)) {
+        RMWI.setOperation(Op == AtomicRMWInst::Add ? AtomicRMWInst::Sub
+                                                   : AtomicRMWInst::Add);
+        return replaceOperand(RMWI, 1, X);
+      }
     }
   }
 
diff --git a/llvm/test/CodeGen/X86/atomicrmw-add-neg.ll b/llvm/test/CodeGen/X86/atomicrmw-add-neg.ll
new file mode 100644
index 0000000000000..a367937ed849b
--- /dev/null
+++ b/llvm/test/CodeGen/X86/atomicrmw-add-neg.ll
@@ -0,0 +1,59 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -passes=instcombine -S %s | llc -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+
+; fetch_add(-n) == n, unsigned: canonicalized to atomicrmw sub, enabling lock sub; sete
+define i1 @fn_add(ptr %a, i64 %n) {
+; CHECK-LABEL: fn_add:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock subq %rsi, (%rdi)
+; CHECK-NEXT:    sete %al
+; CHECK-NEXT:    retq
+  %neg = sub i64 0, %n
+  %old = atomicrmw add ptr %a, i64 %neg monotonic, align 8
+  %cmp = icmp eq i64 %old, %n
+  ret i1 %cmp
+}
+
+; fetch_add(-n) == n, signed nsw: same win as unsigned
+define i1 @fn_adds(ptr %a, i64 %n) {
+; CHECK-LABEL: fn_adds:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock subq %rsi, (%rdi)
+; CHECK-NEXT:    sete %al
+; CHECK-NEXT:    retq
+  %neg = sub nsw i64 0, %n
+  %old = atomicrmw add ptr %a, i64 %neg monotonic, align 8
+  %cmp = icmp eq i64 %old, %n
+  ret i1 %cmp
+}
+
+; fetch_sub(-n), result discarded: canonicalized to atomicrmw add, enabling lock add
+define void @fn_sub_neg(ptr %a, i64 %n) {
+; CHECK-LABEL: fn_sub_neg:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock addq %rsi, (%rdi)
+; CHECK-NEXT:    retq
+  %neg = sub i64 0, %n
+  atomicrmw sub ptr %a, i64 %neg monotonic, align 8
+  ret void
+}
+
+; fetch_add(-10), with discarded result: canonicalized to atomicrmw sub with positive constant
+define void @fn_add_neg_const(ptr %a) {
+; CHECK-LABEL: fn_add_neg_const:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock subq $10, (%rdi)
+; CHECK-NEXT:    retq
+  atomicrmw add ptr %a, i64 -10 monotonic, align 8
+  ret void
+}
+
+; fetch_sub(-10), with discarded result: canonicalized to atomicrmw add with positive constant
+define void @fn_sub_neg_const(ptr %a) {
+; CHECK-LABEL: fn_sub_neg_const:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    lock addq $10, (%rdi)
+; CHECK-NEXT:    retq
+  atomicrmw sub ptr %a, i64 -10 monotonic, align 8
+  ret void
+}
diff --git a/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll b/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
index 9010bdb166bfc..e66e86a20886b 100644
--- a/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
+++ b/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
@@ -1,12 +1,12 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
 ; RUN: opt -passes=instcombine -S %s | FileCheck %s
 
-; Regression case: atomicrmw sub is unchanged by this transform.
+; Regression case: atomicrmw sub is left unchanged
 define i64 @fn_sub(ptr %a, i64 %n) {
 ; CHECK-LABEL: define i64 @fn_sub(
 ; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
-; CHECK-NEXT:    [[TMP0:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
-; CHECK-NEXT:    ret i64 [[TMP0]]
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
+; CHECK-NEXT:    ret i64 [[TMP1]]
 ;
   %1 = atomicrmw sub ptr %a, i64 %n monotonic
   ret i64 %1
@@ -16,15 +16,16 @@ define i64 @fn_sub(ptr %a, i64 %n) {
 define i64 @fn_add(ptr %a, i64 %n) {
 ; CHECK-LABEL: define i64 @fn_add(
 ; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
-; CHECK-NEXT:    [[TMP0:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
-; CHECK-NEXT:    ret i64 [[TMP0]]
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw sub ptr [[A]], i64 [[N]] monotonic, align 8
+; CHECK-NEXT:    ret i64 [[TMP1]]
 ;
   %sub = sub i64 0, %n
   %1 = atomicrmw add ptr %a, i64 %sub monotonic
   ret i64 %1
 }
 
-; nsw neg (signed -n) is also handled; the transform replaces the poison case on neg(INT_MIN) with wrapping sub
+; Canonicalize atomicrmw add(ptr, neg(n)) -> atomicrmw sub(ptr, n)
+; even when %n is signed (nsw)
 define i64 @fn_adds(ptr %a, i64 %n) {
 ; CHECK-LABEL: define i64 @fn_adds(
 ; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
@@ -35,3 +36,37 @@ define i64 @fn_adds(ptr %a, i64 %n) {
   %old = atomicrmw add ptr %a, i64 %neg monotonic
   ret i64 %old
 }
+
+; Canonicalize atomicrmw sub(ptr, neg(n)) -> atomicrmw add(ptr, n)
+define void @fn_sub_neg(ptr %a, i64 %n) {
+; CHECK-LABEL: define void @fn_sub_neg(
+; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw add ptr [[A]], i64 [[N]] monotonic, align 8
+; CHECK-NEXT:    ret void
+;
+  %neg = sub i64 0, %n
+  atomicrmw sub ptr %a, i64 %neg monotonic
+  ret void
+}
+
+; Canonicalize atomicrmw add(ptr, neg(constant)) -> atomicrmw sub(ptr, constant)
+define void @fn_add_neg_const(ptr %a) {
+; CHECK-LABEL: define void @fn_add_neg_const(
+; CHECK-SAME: ptr [[A:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw sub ptr [[A]], i64 10 monotonic, align 8
+; CHECK-NEXT:    ret void
+;
+  atomicrmw add ptr %a, i64 -10 monotonic
+  ret void
+}
+
+; Canonicalize atomicrmw sub(ptr, neg(constant)) -> atomicrmw add(ptr, constant)
+define void @fn_sub_neg_const(ptr %a) {
+; CHECK-LABEL: define void @fn_sub_neg_const(
+; CHECK-SAME: ptr [[A:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw add ptr [[A]], i64 10 monotonic, align 8
+; CHECK-NEXT:    ret void
+;
+  atomicrmw sub ptr %a, i64 -10 monotonic
+  ret void
+}

>From 6d8b2cf47d2396e7f05ae0d8eb2447c98801a5fe Mon Sep 17 00:00:00 2001
From: Takashiidobe <idobetakashi at gmail.com>
Date: Sat, 21 Mar 2026 07:24:56 -0400
Subject: [PATCH 4/4] code review feedback: set LHSIsZero=false in
 Negator::Negate while canonicalizing atomicrmw

Add test for multi-use case where canonicalization should not fire to
avoid adding an extra instruction. Stopped handling constants in this
new transform since they're handled elsewhere.
---
 .../InstCombine/InstCombineAtomicRMW.cpp      |  8 ++-
 llvm/test/CodeGen/X86/atomicrmw-add-neg.ll    | 59 -------------------
 .../InstCombine/atomicrmw-add-neg.ll          | 31 ++++------
 3 files changed, 17 insertions(+), 81 deletions(-)
 delete mode 100644 llvm/test/CodeGen/X86/atomicrmw-add-neg.ll

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
index 2c4b74a2652cc..bf9cd6612925d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
@@ -121,12 +121,14 @@ Instruction *InstCombinerImpl::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
   // Canonicalize atomicrmw add(ptr, neg(X)) -> atomicrmw sub(ptr, X)
   //              atomicrmw sub(ptr, neg(X)) -> atomicrmw add(ptr, X)
   // old + (-X) == old - X and old - (-X) == old + X; the returned old value
-  // is identical in both cases.
+  // is identical in both cases. We only do this for non-constant X; constants
+  // are already properly handled elsewhere.
   auto Op = RMWI.getOperation();
   if (Op == AtomicRMWInst::Add || Op == AtomicRMWInst::Sub) {
     Value *Val = RMWI.getValOperand();
-    if (!isa<Constant>(Val) || match(Val, PatternMatch::m_Negative())) {
-      if (Value *X = Negator::Negate(true, false, Val, *this)) {
+    if (!isa<Constant>(Val)) {
+      if (Value *X = Negator::Negate(/*LHSIsZero=*/false, /*IsNSW=*/false, Val,
+                                     *this)) {
         RMWI.setOperation(Op == AtomicRMWInst::Add ? AtomicRMWInst::Sub
                                                    : AtomicRMWInst::Add);
         return replaceOperand(RMWI, 1, X);
diff --git a/llvm/test/CodeGen/X86/atomicrmw-add-neg.ll b/llvm/test/CodeGen/X86/atomicrmw-add-neg.ll
deleted file mode 100644
index a367937ed849b..0000000000000
--- a/llvm/test/CodeGen/X86/atomicrmw-add-neg.ll
+++ /dev/null
@@ -1,59 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
-; RUN: opt -passes=instcombine -S %s | llc -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
-
-; fetch_add(-n) == n, unsigned: canonicalized to atomicrmw sub, enabling lock sub; sete
-define i1 @fn_add(ptr %a, i64 %n) {
-; CHECK-LABEL: fn_add:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    lock subq %rsi, (%rdi)
-; CHECK-NEXT:    sete %al
-; CHECK-NEXT:    retq
-  %neg = sub i64 0, %n
-  %old = atomicrmw add ptr %a, i64 %neg monotonic, align 8
-  %cmp = icmp eq i64 %old, %n
-  ret i1 %cmp
-}
-
-; fetch_add(-n) == n, signed nsw: same win as unsigned
-define i1 @fn_adds(ptr %a, i64 %n) {
-; CHECK-LABEL: fn_adds:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    lock subq %rsi, (%rdi)
-; CHECK-NEXT:    sete %al
-; CHECK-NEXT:    retq
-  %neg = sub nsw i64 0, %n
-  %old = atomicrmw add ptr %a, i64 %neg monotonic, align 8
-  %cmp = icmp eq i64 %old, %n
-  ret i1 %cmp
-}
-
-; fetch_sub(-n), result discarded: canonicalized to atomicrmw add, enabling lock add
-define void @fn_sub_neg(ptr %a, i64 %n) {
-; CHECK-LABEL: fn_sub_neg:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    lock addq %rsi, (%rdi)
-; CHECK-NEXT:    retq
-  %neg = sub i64 0, %n
-  atomicrmw sub ptr %a, i64 %neg monotonic, align 8
-  ret void
-}
-
-; fetch_add(-10), with discarded result: canonicalized to atomicrmw sub with positive constant
-define void @fn_add_neg_const(ptr %a) {
-; CHECK-LABEL: fn_add_neg_const:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    lock subq $10, (%rdi)
-; CHECK-NEXT:    retq
-  atomicrmw add ptr %a, i64 -10 monotonic, align 8
-  ret void
-}
-
-; fetch_sub(-10), with discarded result: canonicalized to atomicrmw add with positive constant
-define void @fn_sub_neg_const(ptr %a) {
-; CHECK-LABEL: fn_sub_neg_const:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    lock addq $10, (%rdi)
-; CHECK-NEXT:    retq
-  atomicrmw sub ptr %a, i64 -10 monotonic, align 8
-  ret void
-}
diff --git a/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll b/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
index e66e86a20886b..3276be09bd98f 100644
--- a/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
+++ b/llvm/test/Transforms/InstCombine/atomicrmw-add-neg.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
 ; RUN: opt -passes=instcombine -S %s | FileCheck %s
 
-; Regression case: atomicrmw sub is left unchanged
+; Regression case: atomicrmw sub with variable is left unchanged
 define i64 @fn_sub(ptr %a, i64 %n) {
 ; CHECK-LABEL: define i64 @fn_sub(
 ; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
@@ -25,7 +25,7 @@ define i64 @fn_add(ptr %a, i64 %n) {
 }
 
 ; Canonicalize atomicrmw add(ptr, neg(n)) -> atomicrmw sub(ptr, n)
-; even when %n is signed (nsw)
+; even when the negation is nsw
 define i64 @fn_adds(ptr %a, i64 %n) {
 ; CHECK-LABEL: define i64 @fn_adds(
 ; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
@@ -49,24 +49,17 @@ define void @fn_sub_neg(ptr %a, i64 %n) {
   ret void
 }
 
-; Canonicalize atomicrmw add(ptr, neg(constant)) -> atomicrmw sub(ptr, constant)
-define void @fn_add_neg_const(ptr %a) {
-; CHECK-LABEL: define void @fn_add_neg_const(
-; CHECK-SAME: ptr [[A:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw sub ptr [[A]], i64 10 monotonic, align 8
-; CHECK-NEXT:    ret void
-;
-  atomicrmw add ptr %a, i64 -10 monotonic
-  ret void
-}
-
-; Canonicalize atomicrmw sub(ptr, neg(constant)) -> atomicrmw add(ptr, constant)
-define void @fn_sub_neg_const(ptr %a) {
-; CHECK-LABEL: define void @fn_sub_neg_const(
-; CHECK-SAME: ptr [[A:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw add ptr [[A]], i64 10 monotonic, align 8
+; Don't canonicalize if the negated value has multiple uses -- that would add an extra instruction.
+define void @fn_add_1_with_use(ptr %a, i64 %n) {
+; CHECK-LABEL: define void @fn_add_1_with_use(
+; CHECK-SAME: ptr [[A:%.*]], i64 [[N:%.*]]) {
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[N]], 1
+; CHECK-NEXT:    call void @use(i64 [[ADD]])
+; CHECK-NEXT:    [[TMP1:%.*]] = atomicrmw add ptr [[A]], i64 [[ADD]] monotonic, align 8
 ; CHECK-NEXT:    ret void
 ;
-  atomicrmw sub ptr %a, i64 -10 monotonic
+  %add = add i64 %n, 1
+  call void @use(i64 %add)
+  atomicrmw add ptr %a, i64 %add monotonic
   ret void
 }



More information about the llvm-commits mailing list