[llvm] [SelectionDAG] fold (not (sub Y, X)) -> (add X, ~Y) (PR #147825)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 26 11:14:57 PDT 2025
https://github.com/AZero13 updated https://github.com/llvm/llvm-project/pull/147825
>From e05a560931eb64d955a2381c2c88256a710869c1 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Wed, 9 Jul 2025 21:04:24 -0400
Subject: [PATCH 1/3] [SelectionDAG] fold (not (sub Y, X)) -> (add X, ~Y)
This replaces (not (neg x)) -> (add X, -1) because that is covered by this.
---
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 9ffdda28f7899..b74903564f785 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -9979,13 +9979,15 @@ SDValue DAGCombiner::visitXOR(SDNode *N) {
}
}
- // fold (not (neg x)) -> (add X, -1)
- // FIXME: This can be generalized to (not (sub Y, X)) -> (add X, ~Y) if
- // Y is a constant or the subtract has a single use.
- if (isAllOnesConstant(N1) && N0.getOpcode() == ISD::SUB &&
- isNullConstant(N0.getOperand(0))) {
- return DAG.getNode(ISD::ADD, DL, VT, N0.getOperand(1),
- DAG.getAllOnesConstant(DL, VT));
+ // fold (not (sub Y, X)) -> (add X, ~Y) if Y is a constant.
+ // FIXME: We can also do this with single-use sub, but this causes an infinite
+ // loop
+ if (isAllOnesConstant(N1) && N0.getOpcode() == ISD::SUB) {
+ SDValue N00 = N0.getOperand(0), N01 = N0.getOperand(1);
+ if (isa<ConstantSDNode>(N00)) {
+ SDValue NotY = DAG.getNOT(DL, N00, VT); // N00 = ~N00
+ return DAG.getNode(ISD::ADD, DL, VT, N01, NotY);
+ }
}
// fold (not (add X, -1)) -> (neg X)
>From e865959c68c66c90d863a4b420c317ce43ad4b4d Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Sat, 26 Jul 2025 13:22:07 -0400
Subject: [PATCH 2/3] Allow fold to be on platforms that do not have and-not
Otherwise we will end up in an infinite loop. Yes, this should in theory not be whenever there is or-not, and eor-not, or whatever, but in practice this is only an issue for aarch64, and that simply has the same rules as and-not.
---
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index b74903564f785..7082846fdfced 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -9980,12 +9980,19 @@ SDValue DAGCombiner::visitXOR(SDNode *N) {
}
// fold (not (sub Y, X)) -> (add X, ~Y) if Y is a constant.
- // FIXME: We can also do this with single-use sub, but this causes an infinite
- // loop
- if (isAllOnesConstant(N1) && N0.getOpcode() == ISD::SUB) {
+ // fold (not (sub Y, X)) -> (add X, ~Y) for targets with efficient AND-NOT
+ // when Y is single-use and not constant.
+ // FIXME: We cannot do this whenever the target has a hasAndNot, or any other
+ // instruction where the backend will prefer that. Currently, this is only
+ // really a problem for ARM and AArch64, which will have the same rules for
+ // when it has And Not for, in specifically AArch64's case, also xor-not and
+ // or-not Otherwise we end up with an infinite loop.
+ if (N0.getOpcode() == ISD::SUB && isAllOnesConstant(N1) && N0.hasOneUse()) {
SDValue N00 = N0.getOperand(0), N01 = N0.getOperand(1);
- if (isa<ConstantSDNode>(N00)) {
- SDValue NotY = DAG.getNOT(DL, N00, VT); // N00 = ~N00
+ if (isa<ConstantSDNode>(N00) || (N00.hasOneUse() && !TLI.hasAndNot(N01))) {
+ SDValue NotY =
+ DAG.getNode(ISD::XOR, SDLoc(N00), VT, N00, N1); // N00 = ~N00
+ AddToWorklist(NotY.getNode());
return DAG.getNode(ISD::ADD, DL, VT, N01, NotY);
}
}
>From b250b405f91170fd3a1354bf03af6cd0d281f293 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Sat, 26 Jul 2025 14:14:47 -0400
Subject: [PATCH 3/3] Fix comment and test
---
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 7 +++++--
llvm/test/CodeGen/X86/pr31045.ll | 8 ++++----
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 7082846fdfced..ae0c0e52c7ba6 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -9985,9 +9985,12 @@ SDValue DAGCombiner::visitXOR(SDNode *N) {
// FIXME: We cannot do this whenever the target has a hasAndNot, or any other
// instruction where the backend will prefer that. Currently, this is only
// really a problem for ARM and AArch64, which will have the same rules for
- // when it has And Not for, in specifically AArch64's case, also xor-not and
+ // when it has And Not-Or, in specifically AArch64's case, also xor-not and
// or-not Otherwise we end up with an infinite loop.
- if (N0.getOpcode() == ISD::SUB && isAllOnesConstant(N1) && N0.hasOneUse()) {
+ // This is very conservative, and prevents this from being done at all on ARM
+ // and AArch64 and possibly others, but without this check, they will go in an
+ // infinite loop.
+ if (N0.getOpcode() == ISD::SUB && isAllOnesConstant(N1)) {
SDValue N00 = N0.getOperand(0), N01 = N0.getOperand(1);
if (isa<ConstantSDNode>(N00) || (N00.hasOneUse() && !TLI.hasAndNot(N01))) {
SDValue NotY =
diff --git a/llvm/test/CodeGen/X86/pr31045.ll b/llvm/test/CodeGen/X86/pr31045.ll
index 4aa73d79d8cfc..a873fc6fd4959 100644
--- a/llvm/test/CodeGen/X86/pr31045.ll
+++ b/llvm/test/CodeGen/X86/pr31045.ll
@@ -21,11 +21,11 @@ define void @_Z1av() local_unnamed_addr #0 {
; CHECK-NEXT: movl struct_obj_3+8(%rip), %eax
; CHECK-NEXT: movzbl var_46(%rip), %ecx
; CHECK-NEXT: movzbl var_49(%rip), %edx
-; CHECK-NEXT: andl $1, %eax
-; CHECK-NEXT: addl %eax, %eax
-; CHECK-NEXT: subl %ecx, %eax
-; CHECK-NEXT: subl %edx, %eax
+; CHECK-NEXT: addl %ecx, %edx
; CHECK-NEXT: notl %eax
+; CHECK-NEXT: addl %eax, %eax
+; CHECK-NEXT: orl $253, %eax
+; CHECK-NEXT: addl %edx, %eax
; CHECK-NEXT: movzbl %al, %eax
; CHECK-NEXT: movw %ax, struct_obj_12+5(%rip)
; CHECK-NEXT: movb $0, var_163(%rip)
More information about the llvm-commits
mailing list