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

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added a comment.

In D125079#3501078 <https://reviews.llvm.org/D125079#3501078>, @MatzeB wrote:

> 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?

The addtionaly check only happens when it is self loop block, and there are multiple def in current MBB, so I think it won't cause the compiling-time regression for fastregalloc. I am developing regalloc for AMX registers (D125075 <https://reviews.llvm.org/D125075>), it would spill the AMX register before fastregalloc, but here fastregalloc spill it again, so I'd like to improve it.



================
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;
+      }
+    }
----------------
MatzeB wrote:
> This is ignoring any definitions outside of `MBB` but we really have to abort for correctness, don't we?
Update the patch, if the MBB of def is not the same to current MBB mark the VirtReg may liv acorss blocks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125079