Skip to content

Conversation

wlei-llvm
Copy link
Contributor

Merge the two checks using the existing API, NFC.

@llvmbot
Copy link
Member

llvmbot commented Jun 21, 2025

@llvm/pr-subscribers-llvm-regalloc

Author: Lei Wang (wlei-llvm)

Changes

Merge the two checks using the existing API, NFC.


Full diff: https://github.com/llvm/llvm-project/pull/145127.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/MachineSink.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/RegisterPressure.cpp (+1-1)
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());

void RegPressureTracker::recede(SmallVectorImpl<VRegMaskOrUnit> *LiveUses) {
recedeSkipDebugValues();
if (CurrPos->isDebugInstr() || CurrPos->isPseudoProbe()) {
if (CurrPos->isDebugOrPseudoInstr()) {
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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!

Copy link
Member

@WenleiHe WenleiHe left a 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.

@arsenm
Copy link
Contributor

arsenm commented Oct 11, 2025

probably should still go ahead with the rename?

@arsenm arsenm merged commit 8a598f1 into llvm:main Oct 11, 2025
10 checks passed
DharuniRAcharya pushed a commit to DharuniRAcharya/llvm-project that referenced this pull request Oct 13, 2025
Merge the two checks using the existing API, NFC.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants