LiveRangeShrink: Early exit when encountering a code motion barrier.

[pcc]

Without this, we end up with quadratic behavior affecting functions with
large numbers of code motion barriers, such as CFI jump tables.

As a drive-by cleanup, remove a redundant store to SawStore in this
pass as it is also done by isSafeToMove.

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc

Author: Peter Collingbourne (pcc)

Changes

Without this, we end up with quadratic behavior affecting functions with
large numbers of code motion barriers, such as CFI jump tables.

---

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

1 Files Affected:

• (modified) llvm/lib/CodeGen/LiveRangeShrink.cpp (+12-3)

diff --git a/llvm/lib/CodeGen/LiveRangeShrink.cpp b/llvm/lib/CodeGen/LiveRangeShrink.cpp
index dae7e14e54aae..46e837c086c5d 100644
--- a/llvm/lib/CodeGen/LiveRangeShrink.cpp
+++ b/llvm/lib/CodeGen/LiveRangeShrink.cpp
@@ -95,14 +95,24 @@ static MachineInstr *FindDominatedInstruction(MachineInstr &New,
   return Old;
 }
 
+static bool isCodeMotionBarrier(MachineInstr &MI) {
+  return MI.hasUnmodeledSideEffects() && !MI.isPseudoProbe();
+}
+
 /// Builds Instruction to its dominating order number map \p M by traversing
 /// from instruction \p Start.
 static void BuildInstOrderMap(MachineBasicBlock::iterator Start,
                               InstOrderMap &M) {
   M.clear();
   unsigned i = 0;
-  for (MachineInstr &I : make_range(Start, Start->getParent()->end()))
+  bool SawStore = false;
+  for (MachineInstr &I : make_range(Start, Start->getParent()->end())) {
+    if (I.mayStore())
+      SawStore = true;
+    if (!I.isSafeToMove(SawStore) && isCodeMotionBarrier(I))
+      break;
     M[&I] = i++;
+  }
 }
 
 bool LiveRangeShrink::runOnMachineFunction(MachineFunction &MF) {
@@ -166,8 +176,7 @@ bool LiveRangeShrink::runOnMachineFunction(MachineFunction &MF) {
         // If MI has side effects, it should become a barrier for code motion.
         // IOM is rebuild from the next instruction to prevent later
         // instructions from being moved before this MI.
-        if (MI.hasUnmodeledSideEffects() && !MI.isPseudoProbe() &&
-            Next != MBB.end()) {
+        if (isCodeMotionBarrier(MI) && Next != MBB.end()) {
           BuildInstOrderMap(Next, IOM);
           SawStore = false;
         }

[arsenm]

Document this? What is the exact problematic condition? We have several different barrier concepts already spread in MachineInstr and TargetInstrInfo, can we use one of those

[pcc]

Added a comment that describes the situation to the best of my understanding. I don't think the existing ones can be used because the condition is specific to what this pass is doing.

[arsenm]

The MI.hasUnmodeledSideEffects() is redundant with the isSafeToMove check?

[pcc]

Correct, updated this code to only call one.

While reading the code I observed that the code to update SawStore in this pass is redundant with what isSafeToMove is doing, so I removed it.