[AArch64] Allow peephole to optimize AND + signed compare with 0 #153608
Conversation
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
Member
|
@llvm/pr-subscribers-backend-aarch64 Author: AZero13 (AZero13) ChangesThis should be the peephole's job. Because and sets V flag to 0, this is why signed comparisons with 0 are okay to replace with tst. Note this is only for AArch64, because ANDS on ARM leaves the V flag the same. Full diff: https://github.com/llvm/llvm-project/pull/153608.diff 3 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index a55f103bff385..1d909600413aa 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1433,6 +1433,34 @@ static unsigned convertToNonFlagSettingOpc(const MachineInstr &MI) {
return MIDefinesZeroReg ? AArch64::SUBSXrs : AArch64::SUBXrs;
case AArch64::SUBSXrx:
return AArch64::SUBXrx;
+ case AArch64::ANDSWri:
+ return AArch64::ANDWri;
+ case AArch64::ANDSWrr:
+ return AArch64::ANDWrr;
+ case AArch64::ANDSWrs:
+ return AArch64::ANDWrs;
+ case AArch64::ANDSXri:
+ return AArch64::ANDXri;
+ case AArch64::ANDSXrr:
+ return AArch64::ANDXrr;
+ case AArch64::ANDSXrs:
+ return AArch64::ANDXrs;
+ case AArch64::BICSWrr:
+ return AArch64::BICWrr;
+ case AArch64::BICSXrr:
+ return AArch64::BICXrr;
+ case AArch64::BICSWrs:
+ return AArch64::BICWrs;
+ case AArch64::BICSXrs:
+ return AArch64::BICXrs;
+ case AArch64::SBCSXr:
+ return AArch64::SBCXr;
+ case AArch64::SBCSWr:
+ return AArch64::SBCWr;
+ case AArch64::ADCSXr:
+ return AArch64::ADCXr;
+ case AArch64::ADCSWr:
+ return AArch64::ADCWr;
}
}
@@ -1717,6 +1745,16 @@ static unsigned sForm(MachineInstr &Instr) {
case AArch64::SUBSWri:
case AArch64::SUBSXrr:
case AArch64::SUBSXri:
+ case AArch64::ANDSWri:
+ case AArch64::ANDSWrr:
+ case AArch64::ANDSWrs:
+ case AArch64::ANDSXri:
+ case AArch64::ANDSXrr:
+ case AArch64::ANDSXrs:
+ case AArch64::BICSWrr:
+ case AArch64::BICSXrr:
+ case AArch64::BICSWrs:
+ case AArch64::BICSXrs:
return Instr.getOpcode();
case AArch64::ADDWrr:
@@ -1747,6 +1785,22 @@ static unsigned sForm(MachineInstr &Instr) {
return AArch64::ANDSWri;
case AArch64::ANDXri:
return AArch64::ANDSXri;
+ case AArch64::ANDWrr:
+ return AArch64::ANDSWrr;
+ case AArch64::ANDWrs:
+ return AArch64::ANDSWrs;
+ case AArch64::ANDXrr:
+ return AArch64::ANDSXrr;
+ case AArch64::ANDXrs:
+ return AArch64::ANDSXrs;
+ case AArch64::BICWrr:
+ return AArch64::BICSWrr;
+ case AArch64::BICXrr:
+ return AArch64::BICSXrr;
+ case AArch64::BICWrs:
+ return AArch64::BICSWrs;
+ case AArch64::BICXrs:
+ return AArch64::BICSXrs;
}
}
@@ -1884,6 +1938,26 @@ static bool isSUBSRegImm(unsigned Opcode) {
return Opcode == AArch64::SUBSWri || Opcode == AArch64::SUBSXri;
}
+
+static bool isANDOpcode(MachineInstr &MI) {
+ unsigned Opc = sForm(MI);
+ switch (Opc) {
+ case AArch64::ANDSWri:
+ case AArch64::ANDSWrr:
+ case AArch64::ANDSWrs:
+ case AArch64::ANDSXri:
+ case AArch64::ANDSXrr:
+ case AArch64::ANDSXrs:
+ case AArch64::BICSWrr:
+ case AArch64::BICSXrr:
+ case AArch64::BICSWrs:
+ case AArch64::BICSXrs:
+ return true;
+ default:
+ return false;
+ }
+}
+
/// Check if CmpInstr can be substituted by MI.
///
/// CmpInstr can be substituted:
@@ -1921,7 +1995,7 @@ static bool canInstrSubstituteCmpInstr(MachineInstr &MI, MachineInstr &CmpInstr,
// 1) MI and CmpInstr set N and V to the same value.
// 2) If MI is add/sub with no-signed-wrap, it produces a poison value when
// signed overflow occurs, so CmpInstr could still be simplified away.
- if (NZVCUsed->V && !MI.getFlag(MachineInstr::NoSWrap))
+ if (NZVCUsed->V && !MI.getFlag(MachineInstr::NoSWrap) && !isANDOpcode(MI))
return false;
AccessKind AccessToCheck = AK_Write;
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
index 4bd025da636ca..318346c2c8aec 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
@@ -87,6 +87,26 @@ unsigned getNonFlagSettingVariant(unsigned Opc) {
return AArch64::ADDXri;
case AArch64::ADDSWri:
return AArch64::ADDWri;
+ case AArch64::ANDSWri:
+ return AArch64::ANDWri;
+ case AArch64::ANDSWrr:
+ return AArch64::ANDWrr;
+ case AArch64::ANDSWrs:
+ return AArch64::ANDWrs;
+ case AArch64::ANDSXri:
+ return AArch64::ANDXri;
+ case AArch64::ANDSXrr:
+ return AArch64::ANDXrr;
+ case AArch64::ANDSXrs:
+ return AArch64::ANDXrs;
+ case AArch64::BICSWrr:
+ return AArch64::BICWrr;
+ case AArch64::BICSXrr:
+ return AArch64::BICXrr;
+ case AArch64::BICSWrs:
+ return AArch64::BICWrs;
+ case AArch64::BICSXrs:
+ return AArch64::BICXrs;
case AArch64::SBCSXr:
return AArch64::SBCXr;
case AArch64::SBCSWr:
diff --git a/llvm/test/CodeGen/AArch64/arm64-regress-opt-cmp-signed.mir b/llvm/test/CodeGen/AArch64/arm64-regress-opt-cmp-signed.mir
new file mode 100644
index 0000000000000..27a8f91f30a8b
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/arm64-regress-opt-cmp-signed.mir
@@ -0,0 +1,41 @@
+# RUN: llc -mtriple=aarch64-linux-gnu -run-pass peephole-opt -o - %s | FileCheck %s
+# CHECK: %1:gpr32common = ANDSWri {{.*}}
+# CHECK-NOT: $wzr = SUBSWri {{.*}}
+--- |
+ define i32 @test01() nounwind {
+ entry:
+ %0 = select i1 true, i32 1, i32 0
+ %1 = and i32 %0, 65535
+ %2 = icmp sgt i32 %1, 0
+ br i1 %2, label %if.then, label %if.end
+
+ if.then: ; preds = %entry
+ ret i32 1
+
+ if.end: ; preds = %entry
+ ret i32 0
+ }
+...
+---
+name: test01
+registers:
+ - { id: 0, class: gpr32 }
+ - { id: 1, class: gpr32common }
+body: |
+ bb.0.entry:
+ successors: %bb.2.if.end, %bb.1.if.then
+
+ %0 = MOVi32imm 1
+ %1 = ANDWri killed %1, 15
+ $wzr = SUBSWri killed %1, 0, 0, implicit-def $nzcv
+ Bcc 12, %bb.2.if.end, implicit $nzcv
+
+ bb.1.if.then:
+ $w0 = MOVi32imm 1
+ RET_ReallyLR implicit $w0
+
+ bb.2.if.end:
+ $w0 = MOVi32imm 0
+ RET_ReallyLR implicit $w0
+
+...
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
AZero13
force-pushed
the
ede
branch
3 times, most recently
from
c1453eb to
38d4b65
Compare
Contributor
Author
|
@davemgreen what do you think about this? |
Contributor
Author
|
@MacDue Ping? |
Member
|
I think the utility of your change would be more apparent if you pre-comitted some tests. IIUC this is folding things like: to ? |
Contributor
Author
Yes exactly that's the point |
MacDue
reviewed
Aug 22, 2025
Contributor
Author
|
@MacDue Done! |
AZero13
force-pushed
the
ede
branch
2 times, most recently
from
b080125 to
4add981
Compare
Contributor
Author
|
@davemgreen ping? |
Contributor
Author
|
@davemgreen We cannot fold all cases in SelectionDAG/GISel because freezes |
AZero13
force-pushed
the
ede
branch
2 times, most recently
from
c34dec4 to
1c6a37b
Compare
Contributor
Author
|
@MacDue I addressed the issues. Thank you! |
This should be the peephole's job. Because and sets V flag to 0, this is why signed comparisons with 0 are okay to replace with tst. Note this is only for AArch64, because ANDS on ARM leaves the V flag the same. Fixes: llvm#154387
Contributor
Author
|
Thank you @MacDue can you please merge? |
topperc
reviewed
Nov 11, 2025
| return Opcode == AArch64::SUBSWri || Opcode == AArch64::SUBSXri; | ||
| } | ||
|
|
||
| static bool isANDOpcode(MachineInstr &MI) { |
Collaborator
There was a problem hiding this comment.
Oh I guess sForm requires non-const reference, but that also seems wrong.
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.
This should be the peephole's job. Because and sets V flag to 0, this is why signed comparisons with 0 are okay to replace with tst. Note this is only for AArch64, because ANDS on ARM leaves the V flag the same.
Fixes: #154387