Skip to content

[SelectionDAG] Lowering usub.sat(a, 1) to a - (a != 0) #170076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rez5427 wants to merge 5 commits into
base: main
Choose a base branch
from
+46 −6
Open

[SelectionDAG] Lowering usub.sat(a, 1) to a - (a != 0) #170076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
701bcdf
[SelectionDAG] Lowering usub.sat(a, 1) to a - (a != 0)
Dec 1, 2025
2cc21b2
Update llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
rez5427 Dec 3, 2025
87c6428
Use getZExtOrTrunc
rez5427 Dec 4, 2025
0c20d33
Add getBoolExtOrTrunc and and
rez5427 Dec 4, 2025
ae1bccf
Add freeze, ban vector
rez5427 Dec 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10877,6 +10877,18 @@ SDValue TargetLowering::expandAddSubSat(SDNode *Node, SelectionDAG &DAG) const {
assert(VT == RHS.getValueType() && "Expected operands to be the same type");
assert(VT.isInteger() && "Expected operands to be integers");

// usub.sat(a, 1) -> sub(a, zext(a != 0))
if (Opcode == ISD::USUBSAT && !VT.isVector() && isOneConstant(RHS)) {
LHS = DAG.getFreeze(LHS);
SDValue Zero = DAG.getConstant(0, dl, VT);
EVT BoolVT = getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), VT);
SDValue IsNonZero = DAG.getSetCC(dl, BoolVT, LHS, Zero, ISD::SETNE);
Copy link
Collaborator

@topperc topperc Dec 3, 2025

Choose a reason for hiding this comment

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

Do we need to freeze LHS?

Copy link
Collaborator

@RKSimon RKSimon Dec 4, 2025

Choose a reason for hiding this comment

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

https://alive2.llvm.org/ce/z/XSP_Sj

Copy link
Collaborator

@topperc topperc Dec 5, 2025

Choose a reason for hiding this comment

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

That shows that it works with freeze. Looks like it fails if the freeze is removed?

SDValue Subtrahend = DAG.getBoolExtOrTrunc(IsNonZero, dl, VT, BoolVT);
Subtrahend =
DAG.getNode(ISD::AND, dl, VT, Subtrahend, DAG.getConstant(1, dl, VT));
return DAG.getNode(ISD::SUB, dl, VT, LHS, Subtrahend);
}

// usub.sat(a, b) -> umax(a, b) - b
if (Opcode == ISD::USUBSAT && isOperationLegal(ISD::UMAX, VT)) {
SDValue Max = DAG.getNode(ISD::UMAX, dl, VT, LHS, RHS);
Expand Down
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/AArch64/and-mask-removal.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: and w8, w8, #0xff
; CHECK-SD-NEXT: subs w8, w8, #1
; CHECK-SD-NEXT: csel w0, wzr, w8, lo
; CHECK-SD-NEXT: ands w8, w8, #0xff
; CHECK-SD-NEXT: cset w9, ne
; CHECK-SD-NEXT: sub w0, w8, w9
; CHECK-SD-NEXT: ret
;
; CHECK-GI-LABEL: pr58109:
Expand Down
28 changes: 28 additions & 0 deletions llvm/test/CodeGen/RISCV/usub_sat.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,31 @@ define zeroext i4 @func3(i4 zeroext %x, i4 zeroext %y) nounwind {
%tmp = call i4 @llvm.usub.sat.i4(i4 %x, i4 %y);
ret i4 %tmp;
}

define signext i32 @sat_dec_i32(i32 signext %x) nounwind {
; RV32I-LABEL: sat_dec_i32:
; RV32I: # %bb.0:
; RV32I-NEXT: snez a1, a0
; RV32I-NEXT: sub a0, a0, a1
; RV32I-NEXT: ret
;
; RV64I-LABEL: sat_dec_i32:
; RV64I: # %bb.0:
; RV64I-NEXT: snez a1, a0
; RV64I-NEXT: subw a0, a0, a1
; RV64I-NEXT: ret
;
; RV32IZbb-LABEL: sat_dec_i32:
; RV32IZbb: # %bb.0:
; RV32IZbb-NEXT: snez a1, a0
; RV32IZbb-NEXT: sub a0, a0, a1
; RV32IZbb-NEXT: ret
;
; RV64IZbb-LABEL: sat_dec_i32:
; RV64IZbb: # %bb.0:
; RV64IZbb-NEXT: snez a1, a0
; RV64IZbb-NEXT: subw a0, a0, a1
; RV64IZbb-NEXT: ret
%tmp = call i32 @llvm.usub.sat.i32(i32 %x, i32 1)
Copy link
Contributor

@arsenm arsenm Dec 7, 2025

Choose a reason for hiding this comment

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

Does this need a negative test with a non-1 value?

Copy link
Contributor Author

@rez5427 rez5427 Dec 8, 2025

Choose a reason for hiding this comment

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

You mean %y is a constant but not 1?

ret i32 %tmp
}
Copy link
Contributor

@arsenm arsenm Dec 7, 2025

Choose a reason for hiding this comment

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

Also test vector case

Copy link
Contributor Author

@rez5427 rez5427 Dec 8, 2025

Choose a reason for hiding this comment

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

About the vector case, is this previously mentioned PR enough?

6 changes: 3 additions & 3 deletions llvm/test/CodeGen/X86/combine-sub-usat.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ define <8 x i16> @combine_zero_v8i16(<8 x i16> %a0) {
define i32 @combine_dec_i32(i32 %a0) {
; CHECK-LABEL: combine_dec_i32:
; CHECK: # %bb.0:
; CHECK-NEXT: xorl %eax, %eax
; CHECK-NEXT: subl $1, %edi
; CHECK-NEXT: cmovael %edi, %eax
; CHECK-NEXT: movl %edi, %eax
; CHECK-NEXT: cmpl $1, %edi
; CHECK-NEXT: adcl $-1, %eax
; CHECK-NEXT: retq
%1 = call i32 @llvm.usub.sat.i32(i32 %a0, i32 1)
ret i32 %1
Expand Down
Loading