[PowerPC] Do not rely on MachineSink threshold to optimize CR spilling #128745
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-powerpc Author: Guy David (guy-david) Changesspill_p0_setb.ll and knowCRBitSpill.ll rely on a probability threshold in MachineSink to pull down CRSET/CRUNSET. It's worth to do so anyway and let register allocation decide otherwise. Required by #127666. Full diff: https://github.com/llvm/llvm-project/pull/128745.diff 5 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 9e7893d5c4142..bbbe7e667d73b 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -165,7 +165,7 @@ class TargetInstrInfo : public MCInstrInfo {
/// For a "cheap" instruction which doesn't enable additional sinking,
/// should MachineSink break a critical edge to sink it anyways?
- virtual bool shouldBreakCriticalEdgeToSink(MachineInstr &MI) const {
+ virtual bool shouldBreakCriticalEdgeToSink(const MachineInstr &MI) const {
return false;
}
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index 9b526066fe75b..974db8c918c2e 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -1072,6 +1072,13 @@ Register PPCInstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
return 0;
}
+/// Sink down CodeGen-only, cheap instructions to allow further
+/// optimizations which are only applied intra-block.
+bool PPCInstrInfo::shouldBreakCriticalEdgeToSink(const MachineInstr &MI) const {
+ // These can turn into immediates, see PPCRegisterInfo::lowerCRBitRestore.
+ return MI.getOpcode() == PPC::CRSET || MI.getOpcode() == PPC::CRUNSET;
+}
+
// For opcodes with the ReMaterializable flag set, this function is called to
// verify the instruction is really rematable.
bool PPCInstrInfo::isReallyTriviallyReMaterializable(
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.h b/llvm/lib/Target/PowerPC/PPCInstrInfo.h
index d4554379cdb1d..892a670dbfce1 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.h
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.h
@@ -424,6 +424,7 @@ class PPCInstrInfo : public PPCGenInstrInfo {
unsigned &SubIdx) const override;
Register isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
+ bool shouldBreakCriticalEdgeToSink(const MachineInstr &MI) const override;
bool isReallyTriviallyReMaterializable(const MachineInstr &MI) const override;
Register isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 1c46d761a7e1e..465bbd5dc2b60 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -78,7 +78,7 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
bool isReallyTriviallyReMaterializable(const MachineInstr &MI) const override;
- bool shouldBreakCriticalEdgeToSink(MachineInstr &MI) const override {
+ bool shouldBreakCriticalEdgeToSink(const MachineInstr &MI) const override {
return MI.getOpcode() == RISCV::ADDI && MI.getOperand(1).isReg() &&
MI.getOperand(1).getReg() == RISCV::X0;
}
diff --git a/llvm/test/CodeGen/PowerPC/spill_p9_setb.ll b/llvm/test/CodeGen/PowerPC/spill_p9_setb.ll
index 81c43ce85d59a..b740296e7d710 100644
--- a/llvm/test/CodeGen/PowerPC/spill_p9_setb.ll
+++ b/llvm/test/CodeGen/PowerPC/spill_p9_setb.ll
@@ -16,21 +16,21 @@
define void @p9_setb_spill() {
; CHECK-P9-LABEL: p9_setb_spill:
-; CHECK-P9: # %bb.1: # %if.then
+; CHECK-P9: # %if.then
; CHECK-P9-DAG: crnot 4*cr[[CREG:.*]]+lt, eq
; CHECK-P9-DAG: setb [[REG1:.*]], cr[[CREG]]
; CHECK-P9-DAG: stw [[REG1]]
; CHECK-P9: blr
-; CHECK-P9: .LBB0_4: # %if.then1
+; CHECK-P9: # %if.then1
;
; CHECK-P8-LABEL: p9_setb_spill:
-; CHECK-P8: # %bb.1: # %if.then
+; CHECK-P8: # %if.then
; CHECK-P8-DAG: crnot 4*cr[[CREG2:.*]]+lt, eq
; CHECK-P8-DAG: mfocrf [[REG2:.*]],
; CHECK-P8-DAG: rlwinm [[REG2]], [[REG2]]
; CHECK-P8-DAG: stw [[REG2]]
; CHECK-P8: blr
-; CHECK-P8: .LBB0_4: # %if.then1
+; CHECK-P8: # %if.then1
entry:
br i1 undef, label %if.end, label %if.then
|
Member
|
@llvm/pr-subscribers-backend-risc-v Author: Guy David (guy-david) Changesspill_p0_setb.ll and knowCRBitSpill.ll rely on a probability threshold in MachineSink to pull down CRSET/CRUNSET. It's worth to do so anyway and let register allocation decide otherwise. Required by #127666. Full diff: https://github.com/llvm/llvm-project/pull/128745.diff 5 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 9e7893d5c4142..bbbe7e667d73b 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -165,7 +165,7 @@ class TargetInstrInfo : public MCInstrInfo {
/// For a "cheap" instruction which doesn't enable additional sinking,
/// should MachineSink break a critical edge to sink it anyways?
- virtual bool shouldBreakCriticalEdgeToSink(MachineInstr &MI) const {
+ virtual bool shouldBreakCriticalEdgeToSink(const MachineInstr &MI) const {
return false;
}
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index 9b526066fe75b..974db8c918c2e 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -1072,6 +1072,13 @@ Register PPCInstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
return 0;
}
+/// Sink down CodeGen-only, cheap instructions to allow further
+/// optimizations which are only applied intra-block.
+bool PPCInstrInfo::shouldBreakCriticalEdgeToSink(const MachineInstr &MI) const {
+ // These can turn into immediates, see PPCRegisterInfo::lowerCRBitRestore.
+ return MI.getOpcode() == PPC::CRSET || MI.getOpcode() == PPC::CRUNSET;
+}
+
// For opcodes with the ReMaterializable flag set, this function is called to
// verify the instruction is really rematable.
bool PPCInstrInfo::isReallyTriviallyReMaterializable(
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.h b/llvm/lib/Target/PowerPC/PPCInstrInfo.h
index d4554379cdb1d..892a670dbfce1 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.h
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.h
@@ -424,6 +424,7 @@ class PPCInstrInfo : public PPCGenInstrInfo {
unsigned &SubIdx) const override;
Register isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
+ bool shouldBreakCriticalEdgeToSink(const MachineInstr &MI) const override;
bool isReallyTriviallyReMaterializable(const MachineInstr &MI) const override;
Register isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 1c46d761a7e1e..465bbd5dc2b60 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -78,7 +78,7 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
bool isReallyTriviallyReMaterializable(const MachineInstr &MI) const override;
- bool shouldBreakCriticalEdgeToSink(MachineInstr &MI) const override {
+ bool shouldBreakCriticalEdgeToSink(const MachineInstr &MI) const override {
return MI.getOpcode() == RISCV::ADDI && MI.getOperand(1).isReg() &&
MI.getOperand(1).getReg() == RISCV::X0;
}
diff --git a/llvm/test/CodeGen/PowerPC/spill_p9_setb.ll b/llvm/test/CodeGen/PowerPC/spill_p9_setb.ll
index 81c43ce85d59a..b740296e7d710 100644
--- a/llvm/test/CodeGen/PowerPC/spill_p9_setb.ll
+++ b/llvm/test/CodeGen/PowerPC/spill_p9_setb.ll
@@ -16,21 +16,21 @@
define void @p9_setb_spill() {
; CHECK-P9-LABEL: p9_setb_spill:
-; CHECK-P9: # %bb.1: # %if.then
+; CHECK-P9: # %if.then
; CHECK-P9-DAG: crnot 4*cr[[CREG:.*]]+lt, eq
; CHECK-P9-DAG: setb [[REG1:.*]], cr[[CREG]]
; CHECK-P9-DAG: stw [[REG1]]
; CHECK-P9: blr
-; CHECK-P9: .LBB0_4: # %if.then1
+; CHECK-P9: # %if.then1
;
; CHECK-P8-LABEL: p9_setb_spill:
-; CHECK-P8: # %bb.1: # %if.then
+; CHECK-P8: # %if.then
; CHECK-P8-DAG: crnot 4*cr[[CREG2:.*]]+lt, eq
; CHECK-P8-DAG: mfocrf [[REG2:.*]],
; CHECK-P8-DAG: rlwinm [[REG2]], [[REG2]]
; CHECK-P8-DAG: stw [[REG2]]
; CHECK-P8: blr
-; CHECK-P8: .LBB0_4: # %if.then1
+; CHECK-P8: # %if.then1
entry:
br i1 undef, label %if.end, label %if.then
|
mshockwave
reviewed
Feb 25, 2025
guy-david
force-pushed
the
users/guy-david/machine-sink-powerpc
branch
from
2001146 to
fdcf246
Compare
mshockwave
reviewed
Feb 25, 2025
Member
mshockwave
left a comment
mshockwave
left a comment
There was a problem hiding this comment.
Overall looks good to me. However, I'm not really familiar with the PPC instructions mentioned in this patch, could you tag some PowerPC folks to give this a look too?
guy-david
force-pushed
the
users/guy-david/machine-sink-powerpc
branch
from
fdcf246 to
40b923a
Compare
guy-david
force-pushed
the
users/guy-david/machine-sink-powerpc
branch
from
40b923a to
cebcc2f
Compare
spill_p0_setb.ll and knowCRBitSpill.ll rely on a probability threshold in MachineSink to pull down CRSET/CRUNSET. It's worth to do so anyway and let register allocation decide otherwise.
guy-david
force-pushed
the
users/guy-david/machine-sink-powerpc
branch
from
cebcc2f to
95af9b3
Compare
Contributor
Author
|
Closing because the PR that required this is no longer relevant: #127666. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
spill_p0_setb.ll and knowCRBitSpill.ll rely on a probability threshold in MachineSink to pull down CRSET/CRUNSET. It's worth to do so anyway and let register allocation decide otherwise.
Required by #127666.