-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[NFC] Simplify checks using isDebugOrPseudoInstr API #145127
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.
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-llvm-regalloc Author: Lei Wang (wlei-llvm) ChangesMerge the two checks using the existing API, NFC. Full diff: https://github.com/llvm/llvm-project/pull/145127.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index 8411d5c4b09c8..e9544727af9e4 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -1208,7 +1208,7 @@ MachineSinking::getBBRegisterPressure(const MachineBasicBlock &MBB,
MIE = MBB.instr_begin();
MII != MIE; --MII) {
const MachineInstr &MI = *std::prev(MII);
- if (MI.isDebugInstr() || MI.isPseudoProbe())
+ if (MI.isDebugOrPseudoInstr())
continue;
RegisterOperands RegOpers;
RegOpers.collect(MI, *TRI, *MRI, false, false);
diff --git a/llvm/lib/CodeGen/RegisterPressure.cpp b/llvm/lib/CodeGen/RegisterPressure.cpp
index ca51b670b46cc..bcfa270b1e7c8 100644
--- a/llvm/lib/CodeGen/RegisterPressure.cpp
+++ b/llvm/lib/CodeGen/RegisterPressure.cpp
@@ -858,7 +858,7 @@ void RegPressureTracker::recedeSkipDebugValues() {
void RegPressureTracker::recede(SmallVectorImpl<VRegMaskOrUnit> *LiveUses) {
recedeSkipDebugValues();
- if (CurrPos->isDebugInstr() || CurrPos->isPseudoProbe()) {
+ if (CurrPos->isDebugOrPseudoInstr()) {
// It's possible to only have debug_value and pseudo probe instructions and
// hit the start of the block.
assert(CurrPos == MBB->begin());
|
@@ -858,7 +858,7 @@ void RegPressureTracker::recedeSkipDebugValues() { | |||
|
|||
void RegPressureTracker::recede(SmallVectorImpl<VRegMaskOrUnit> *LiveUses) { | |||
recedeSkipDebugValues(); | |||
if (CurrPos->isDebugInstr() || CurrPos->isPseudoProbe()) { | |||
if (CurrPos->isDebugOrPseudoInstr()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the isDebugOrPseudoInstr name is confusing. We already have multiple notions of "pseudoinstructions" and specifically PseudoProbe isn't one of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, any suggestions? would isDebugOrPseudoProbeInstr
be better?
Would it be ok to check in the current diff first? as the changes are independent of the naming, I'll create a separate diff for the name change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps isDebugInstrOrPseudoProbe
? But since this PR did not introduce new API, I'd suggest leave renaming existing API to a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good with isDebugInstrOrPseudoProbe
, thank for the suggestion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm and suggest deal with renaming of existing API separately.
Merge the two checks using the existing API, NFC.