Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Mar 6, 2025

Instead of scanning the whole basic block for a POP, find the RET and then look backwards for the POP. Using getFirstTerminator, we can do this with less code and it's probably faster.

Instead of scanning the entire basic block for a pop. Find the RET
and then look backwards for the POP. Using getFirstTerminator, we
can do this with less code and its probably faster.
@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

Instead of scanning the whole basic block for a POP, find the RET and then look backwards for the POP. Using getFirstTerminator, we can do this with less code and its probably faster.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp (+14-15)
diff --git a/llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp
index 0ead9a4009fab..eae7e8697f0ad 100644
--- a/llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp
@@ -69,16 +69,6 @@ static unsigned getPopRetOpcode(unsigned PopOpcode, bool IsReturnZero) {
   }
 }
 
-// Check if POP instruction was inserted into the MBB and return iterator to it.
-static MachineBasicBlock::iterator containsPop(MachineBasicBlock &MBB) {
-  for (MachineBasicBlock::iterator MBBI = MBB.begin(); MBBI != MBB.end();
-       MBBI = next_nodbg(MBBI, MBB.end()))
-    if (MBBI->getFlag(MachineInstr::FrameDestroy) && isPop(MBBI->getOpcode()))
-      return MBBI;
-
-  return MBB.end();
-}
-
 bool RISCVPushPopOpt::usePopRet(MachineBasicBlock::iterator &MBBI,
                                 MachineBasicBlock::iterator &NextI,
                                 bool IsReturnZero) {
@@ -150,19 +140,28 @@ bool RISCVPushPopOpt::runOnMachineFunction(MachineFunction &Fn) {
 
   TII = Subtarget->getInstrInfo();
   TRI = Subtarget->getRegisterInfo();
+
   // Resize the modified and used register unit trackers.  We do this once
   // per function and then clear the register units each time we determine
   // correct return value for the POP.
   ModifiedRegUnits.init(*TRI);
   UsedRegUnits.init(*TRI);
+
   bool Modified = false;
   for (auto &MBB : Fn) {
-    MachineBasicBlock::iterator MBBI = containsPop(MBB);
-    MachineBasicBlock::iterator NextI = next_nodbg(MBBI, MBB.end());
-    if (MBBI != MBB.end() && NextI != MBB.end() &&
-        NextI->getOpcode() == RISCV::PseudoRET)
-      Modified |= usePopRet(MBBI, NextI, adjustRetVal(MBBI));
+    // RET should be the only terminator.
+    auto RetMBBI = MBB.getFirstTerminator();
+    if (RetMBBI == MBB.end() || RetMBBI->getOpcode() != RISCV::PseudoRET ||
+        RetMBBI == MBB.begin())
+      continue;
+
+    // The previous instruction should be a POP.
+    auto PopMBBI = prev_nodbg(RetMBBI, MBB.begin());
+    if (isPop(PopMBBI->getOpcode()) &&
+        PopMBBI->getFlag(MachineInstr::FrameDestroy))
+      Modified |= usePopRet(PopMBBI, RetMBBI, adjustRetVal(PopMBBI));
   }
+
   return Modified;
 }
 

@topperc topperc merged commit bedb907 into llvm:main Mar 7, 2025
13 checks passed
@topperc topperc deleted the pr/popret branch March 7, 2025 05:55
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.

3 participants