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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

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

> If I'm understanding correctly, the issue is that the INLINEASM_BR is producing a virtual register, we try to spill that register, and the spill is inserted in the same block as the INLINEASM_BR?
>
> I can see two possible approaches here, at a high level:
>
> 1. Forbid directly spilling registers produced by an INLINEASM_BR; instead, force a live interval split, and then we can spill the split interval.
> 2. Allow spilling registers produced by an INLINEASM_BR, but teach the spill insertion code to insert the spill into the fallthrough succecssor, as opposed to the INLINEASM_BR block itself.


Re: 2: it looks like `InlineSpiller::insertSpill` might be the most appropriate place to do that, as it just picks the next `MachineInstr` to insert the spill before, even in that case it violates the invariant that only `Terminal` `MachineInstr` end a `MachineBasicBlock` (no non-`Terminal` `MachineInstr`s after the first `Terminal` `MachineInstr`).  We could check that `std::next(MI)->getOpcode == TargetOpcode::INLINEASM_BR`, or `!std::next(MI).isTerminator()`, though I'm not sure what to do in the latter case.

That would do less work for the general case; the current approach checks the opcode of every `MachineInstr` that is the source of a `LiveInterval`, which there are generally a lot.  This would only check for the special case when we decided we needed to spill (still occurs frequently for large `MachineFunction`s, but a lot less than the number of `LiveIntervals`.  Let me see if I can update the patch to do that.

Re: 1: I totally do not understand. :( Are you referring to `LiveIntervals::splitSeparateComponents`?

> This patch is sort of along the lines of (1), but I'm not sure this is enough to make the interval split reliably (in which case, the allocator could potentially run out of registers).  I haven't spent enough time with the spill weight code to say for sure.

Is there someone more active in this area you could cc for review?  This bug is blocking the use of `asm goto` w/ outputs in the Linux kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77849