[PATCH] D77849: [calcspillweights] mark LiveIntervals from INLINEASM_BR defs as not spillable

Tue Apr 14 14:05:24 PDT 2020

nickdesaulniers added a comment.

In D77849#1973733 <https://reviews.llvm.org/D77849#1973733>, @efriedma wrote:

> 1. Forbid directly spilling registers produced by an INLINEASM_BR; instead, force a live interval split, and then we can spill the split interval.

In D77849#1975590 <https://reviews.llvm.org/D77849#1975590>, @arsenm wrote:

> Using LiveIntervals in any way is not an option to solve this. They are not available in all fast regalloc and other allocators. The constraints need to be expressed with copies that are terminators, and splitting blocks when necessary

Right, but I'll bet that when FastAlloc needs to spill, it also uses `InlineSpiller` to do so.  I want to play with or prototype to see if we can make `InlineSpiller` not violate `MachineVerifier` invariants by spilling post terminal instructions.

Some other notes I had jotted down, reviewers please speak up if any of these seem immediately not worth pursuing:

1. `branch-folder` should probably `assert` when removing a `MachineBasicBlock` that has its address taken.  That's twice now we've seen this produce completely garbage code.  I don't think we should prevent it, as both times its been a cascading failure, so I think an `assert` is more appropriate.
2. `assert` in `inline-spiller` if it places a spill after a terminator `MachineInstr`.  This will wind up breaking the `MachineVerifier` constraint about non terminal instructions post terminators anyways.  We might even be able to solve this problem there, maybe.
3. "canonicalize" the fallthrough case during our block splitting special case in instruction scheduling to put the fallthrough immediately after the `INLINEASM_BR`'s parent `MachineBasicBlock` (as in this test case, and not any of the existing ones).  I suspect this will help reduce overlapping `LiveRanges` and help reduce register pressure.

And of course https://reviews.llvm.org/D75098.

================
Comment at: llvm/test/CodeGen/X86/callbr-asm-regalloc.mir:5
+#                 -simplify-mir llvm/test/CodeGen/X86/callbr-asm-regalloc.ll \
+#                 2> callbr-asm-regalloc.mir
+#
----------------
arsenm wrote:
> nickdesaulniers wrote:
> > huh, that's not yaml...
> The debug printing format that -print-after gives you is not the same MIR output as -stop-after
Ok, I think I have it updated.  Seems that regalloc is actually a grouping of passes that run, so there's no explicit "Greedy" in `-print-after-all` and you have to dump the MIR before any of the related regalloc passes (ie. after `twoaddressinstruction` for x86).  MIR/llc is different from LLVM IR/opt in that passes generally are order dependent, so you must dump MIR in the precise form that a given pass expects.  Someone please correct me if I'm wrong in my understanding of this point.

I suppose I can first clean up this test by removing the LLVM IR embedded within.

We need a test case for which there are more live registers in use than there are physical GPRs to allocate, and where the register allocator chooses to spill the liveouts from the `INLINEASM_BR`.  @arsenm , I'm not sure how much more I can simplify the test, at least in an automated manner.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77849