[PATCH] D125079: [fastregalloc] Enhance the heuristics for liveout in self loop.

[Matthias Braun via Phabricator via llvm-commits]

MatzeB requested changes to this revision.
MatzeB added a comment.
This revision now requires changes to proceed.

FWIW: This is adding another case of quadratic runtime in the amount of instructions in the basic block: We can potentially hit the check for every instruction in the basic block and then use `dominates` and end up scanning over the majority of the basic block again. We have to be careful because the fast regallocators primary quality is being fast, being good comes second.

That said the changes in the tests indicate that this is common enough that it may be worth having?

Either way in the current form this seems incorrect to me (see comment).



================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:363-368
+    for (const MachineInstr &DefInst : MRI->def_instructions(VirtReg)) {
+      if (DefInst.getParent() == MBB) {
+        if (!SelfLoopDef || dominates(*MBB, DefInst.getIterator(), SelfLoopDef))
+          SelfLoopDef = &DefInst;
+      }
+    }
----------------
This is ignoring any definitions outside of `MBB` but we really have to abort for correctness, don't we?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125079/new/

https://reviews.llvm.org/D125079