[AArch64][SelectionDAG] Generate subs+csel for usub.sat#193203
Open
[AArch64][SelectionDAG] Generate subs+csel for usub.sat#193203
Conversation
Member
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-llvm-selectiondag Author: Shreeyash Pandey (bojle) ChangesFixes #191488 As this is a regression of Change-Id: I0a194bcc9e66819c12d0f9179464823301f0d7bf Full diff: https://github.com/llvm/llvm-project/pull/193203.diff 6 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 59a0f2d2e0c2a..441a407e2edc1 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -3595,6 +3595,10 @@ class LLVM_ABI TargetLoweringBase {
return false;
}
+ /// Should usub.sat(X, 1) prefer the generic lowering X - zext(X != 0) over
+ /// the default overflow/select expansion?
+ virtual bool preferSubOfZextForUsubSatOne(EVT VT) const { return true; }
+
/// True if target has some particular form of dealing with pointer arithmetic
/// semantics for pointers with the given value type. False if pointer
/// arithmetic should not be preserved for passes such as instruction
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index e6aa222425d13..be7401c0328d2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -11475,7 +11475,8 @@ SDValue TargetLowering::expandAddSubSat(SDNode *Node, SelectionDAG &DAG) const {
}
// usub.sat(a, 1) -> sub(a, zext(a != 0))
- if (Opcode == ISD::USUBSAT && isOneOrOneSplat(RHS)) {
+ if (Opcode == ISD::USUBSAT && isOneOrOneSplat(RHS) &&
+ preferSubOfZextForUsubSatOne(VT)) {
LHS = DAG.getFreeze(LHS);
SDValue Zero = DAG.getConstant(0, dl, VT);
EVT BoolVT = getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), VT);
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 9b34d9b385b4e..ea4d5467c73d5 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -30978,6 +30978,11 @@ bool AArch64TargetLowering::shouldConvertFpToSat(unsigned Op, EVT FPVT,
return TargetLowering::shouldConvertFpToSat(Op, FPVT, VT);
}
+bool AArch64TargetLowering::preferSubOfZextForUsubSatOne(EVT /*VT*/) const {
+ // See https://github.com/llvm/llvm-project/issues/191488
+ return false;
+}
+
bool AArch64TargetLowering::preferSelectsOverBooleanArithmetic(EVT VT) const {
// Expand scalar and SVE operations using selects. Neon vectors prefer sub to
// avoid vselect becoming bsl / unrolling.
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.h b/llvm/lib/Target/AArch64/AArch64ISelLowering.h
index 58efdd3e18fc0..cdef09ef7013e 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -450,6 +450,8 @@ class AArch64TargetLowering : public TargetLowering {
bool shouldConvertFpToSat(unsigned Op, EVT FPVT, EVT VT) const override;
+ bool preferSubOfZextForUsubSatOne(EVT VT) const override;
+
bool preferSelectsOverBooleanArithmetic(EVT VT) const override;
bool isComplexDeinterleavingSupported() const override;
diff --git a/llvm/test/CodeGen/AArch64/and-mask-removal.ll b/llvm/test/CodeGen/AArch64/and-mask-removal.ll
index 855fe5caf97b2..5046c0571ad2b 100644
--- a/llvm/test/CodeGen/AArch64/and-mask-removal.ll
+++ b/llvm/test/CodeGen/AArch64/and-mask-removal.ll
@@ -483,9 +483,9 @@ define i64 @pr58109(i8 signext %0) {
; CHECK-SD-LABEL: pr58109:
; CHECK-SD: ; %bb.0:
; CHECK-SD-NEXT: add w8, w0, #1
-; CHECK-SD-NEXT: ands w8, w8, #0xff
-; CHECK-SD-NEXT: cset w9, ne
-; CHECK-SD-NEXT: sub w0, w8, w9
+; CHECK-SD-NEXT: and w8, w8, #0xff
+; CHECK-SD-NEXT: subs w8, w8, #1
+; CHECK-SD-NEXT: csel w0, wzr, w8, lo
; CHECK-SD-NEXT: ret
;
; CHECK-GI-LABEL: pr58109:
diff --git a/llvm/test/CodeGen/AArch64/usub_sat_plus.ll b/llvm/test/CodeGen/AArch64/usub_sat_plus.ll
index 2793aeb163c94..9f1e2eeb04781 100644
--- a/llvm/test/CodeGen/AArch64/usub_sat_plus.ll
+++ b/llvm/test/CodeGen/AArch64/usub_sat_plus.ll
@@ -8,6 +8,24 @@ declare i16 @llvm.usub.sat.i16(i16, i16)
declare i32 @llvm.usub.sat.i32(i32, i32)
declare i64 @llvm.usub.sat.i64(i64, i64)
+define i32 @sat_dec_i32(i32 %x) nounwind {
+; CHECK-SD-LABEL: sat_dec_i32:
+; CHECK-SD: // %bb.0:
+; CHECK-SD-NEXT: subs w8, w0, #1
+; CHECK-SD-NEXT: csel w0, wzr, w8, lo
+; CHECK-SD-NEXT: ret
+;
+; CHECK-GI-LABEL: sat_dec_i32:
+; CHECK-GI: // %bb.0:
+; CHECK-GI-NEXT: subs w8, w0, #1
+; CHECK-GI-NEXT: cset w9, lo
+; CHECK-GI-NEXT: tst w9, #0x1
+; CHECK-GI-NEXT: csel w0, wzr, w8, ne
+; CHECK-GI-NEXT: ret
+ %tmp = call i32 @llvm.usub.sat.i32(i32 %x, i32 1)
+ ret i32 %tmp
+}
+
define i32 @func32(i32 %x, i32 %y, i32 %z) nounwind {
; CHECK-SD-LABEL: func32:
; CHECK-SD: // %bb.0:
|
Member
|
@llvm/pr-subscribers-backend-aarch64 Author: Shreeyash Pandey (bojle) ChangesFixes #191488 As this is a regression of Change-Id: I0a194bcc9e66819c12d0f9179464823301f0d7bf Full diff: https://github.com/llvm/llvm-project/pull/193203.diff 6 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 59a0f2d2e0c2a..441a407e2edc1 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -3595,6 +3595,10 @@ class LLVM_ABI TargetLoweringBase {
return false;
}
+ /// Should usub.sat(X, 1) prefer the generic lowering X - zext(X != 0) over
+ /// the default overflow/select expansion?
+ virtual bool preferSubOfZextForUsubSatOne(EVT VT) const { return true; }
+
/// True if target has some particular form of dealing with pointer arithmetic
/// semantics for pointers with the given value type. False if pointer
/// arithmetic should not be preserved for passes such as instruction
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index e6aa222425d13..be7401c0328d2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -11475,7 +11475,8 @@ SDValue TargetLowering::expandAddSubSat(SDNode *Node, SelectionDAG &DAG) const {
}
// usub.sat(a, 1) -> sub(a, zext(a != 0))
- if (Opcode == ISD::USUBSAT && isOneOrOneSplat(RHS)) {
+ if (Opcode == ISD::USUBSAT && isOneOrOneSplat(RHS) &&
+ preferSubOfZextForUsubSatOne(VT)) {
LHS = DAG.getFreeze(LHS);
SDValue Zero = DAG.getConstant(0, dl, VT);
EVT BoolVT = getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), VT);
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 9b34d9b385b4e..ea4d5467c73d5 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -30978,6 +30978,11 @@ bool AArch64TargetLowering::shouldConvertFpToSat(unsigned Op, EVT FPVT,
return TargetLowering::shouldConvertFpToSat(Op, FPVT, VT);
}
+bool AArch64TargetLowering::preferSubOfZextForUsubSatOne(EVT /*VT*/) const {
+ // See https://github.com/llvm/llvm-project/issues/191488
+ return false;
+}
+
bool AArch64TargetLowering::preferSelectsOverBooleanArithmetic(EVT VT) const {
// Expand scalar and SVE operations using selects. Neon vectors prefer sub to
// avoid vselect becoming bsl / unrolling.
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.h b/llvm/lib/Target/AArch64/AArch64ISelLowering.h
index 58efdd3e18fc0..cdef09ef7013e 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -450,6 +450,8 @@ class AArch64TargetLowering : public TargetLowering {
bool shouldConvertFpToSat(unsigned Op, EVT FPVT, EVT VT) const override;
+ bool preferSubOfZextForUsubSatOne(EVT VT) const override;
+
bool preferSelectsOverBooleanArithmetic(EVT VT) const override;
bool isComplexDeinterleavingSupported() const override;
diff --git a/llvm/test/CodeGen/AArch64/and-mask-removal.ll b/llvm/test/CodeGen/AArch64/and-mask-removal.ll
index 855fe5caf97b2..5046c0571ad2b 100644
--- a/llvm/test/CodeGen/AArch64/and-mask-removal.ll
+++ b/llvm/test/CodeGen/AArch64/and-mask-removal.ll
@@ -483,9 +483,9 @@ define i64 @pr58109(i8 signext %0) {
; CHECK-SD-LABEL: pr58109:
; CHECK-SD: ; %bb.0:
; CHECK-SD-NEXT: add w8, w0, #1
-; CHECK-SD-NEXT: ands w8, w8, #0xff
-; CHECK-SD-NEXT: cset w9, ne
-; CHECK-SD-NEXT: sub w0, w8, w9
+; CHECK-SD-NEXT: and w8, w8, #0xff
+; CHECK-SD-NEXT: subs w8, w8, #1
+; CHECK-SD-NEXT: csel w0, wzr, w8, lo
; CHECK-SD-NEXT: ret
;
; CHECK-GI-LABEL: pr58109:
diff --git a/llvm/test/CodeGen/AArch64/usub_sat_plus.ll b/llvm/test/CodeGen/AArch64/usub_sat_plus.ll
index 2793aeb163c94..9f1e2eeb04781 100644
--- a/llvm/test/CodeGen/AArch64/usub_sat_plus.ll
+++ b/llvm/test/CodeGen/AArch64/usub_sat_plus.ll
@@ -8,6 +8,24 @@ declare i16 @llvm.usub.sat.i16(i16, i16)
declare i32 @llvm.usub.sat.i32(i32, i32)
declare i64 @llvm.usub.sat.i64(i64, i64)
+define i32 @sat_dec_i32(i32 %x) nounwind {
+; CHECK-SD-LABEL: sat_dec_i32:
+; CHECK-SD: // %bb.0:
+; CHECK-SD-NEXT: subs w8, w0, #1
+; CHECK-SD-NEXT: csel w0, wzr, w8, lo
+; CHECK-SD-NEXT: ret
+;
+; CHECK-GI-LABEL: sat_dec_i32:
+; CHECK-GI: // %bb.0:
+; CHECK-GI-NEXT: subs w8, w0, #1
+; CHECK-GI-NEXT: cset w9, lo
+; CHECK-GI-NEXT: tst w9, #0x1
+; CHECK-GI-NEXT: csel w0, wzr, w8, ne
+; CHECK-GI-NEXT: ret
+ %tmp = call i32 @llvm.usub.sat.i32(i32 %x, i32 1)
+ ret i32 %tmp
+}
+
define i32 @func32(i32 %x, i32 %y, i32 %z) nounwind {
; CHECK-SD-LABEL: func32:
; CHECK-SD: // %bb.0:
|
arsenm
reviewed
Apr 21, 2026
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
RKSimon
requested changes
Apr 22, 2026
Collaborator
RKSimon
left a comment
RKSimon
left a comment
There was a problem hiding this comment.
Please regenerate x86/combine-sub-usat.ll
Fixes llvm#191488 As this is a regression of llvm#170076, adds a check to avoid generic lowering of usub.sat to X - zext(X != 0) in case of aarch64 via a virtual hook in TargetLowering. All other backends will still receive generic lowering as implemented in the original patch. Change-Id: I0a194bcc9e66819c12d0f9179464823301f0d7bf
Change-Id: I4575a604fe5a76be2e657c63707c5fe25d631b98
Change-Id: If34cdfe1eea8c30f6be502a63270ae5b6c34d141
Contributor
Author
|
hmm... the CI failures seem unrelated to this patch. Are these the same failures as #193724? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #191488
As this is a regression of
#170076, adds a check to avoid generic lowering of usub.sat to X - zext(X != 0) in case of aarch64 via a virtual hook in TargetLowering. All other backends will still receive generic lowering as implemented in the original patch.
Change-Id: I0a194bcc9e66819c12d0f9179464823301f0d7bf