[PATCH] D75098: Add TCOPY, a terminator form of the COPY instr

[Bill Wendling via Phabricator via llvm-commits]

void marked an inline comment as done.
void added a comment.

In D75098#2017980 <https://reviews.llvm.org/D75098#2017980>, @qcolombet wrote:

> I don't think introducing the TCOPY opcode is a valid solution.
>  Let's assume the copy doesn't get coalesced, that means it needs to be executed after the inlineasm branch. And this is not going to happen if it sits in the same basic block right after the jump.
>
> What happens if you put the copy at the beginning of each successor?
>  You mentioned that this is not possible because the live-ins wouldn't be set, but I would expect that this information would be correctly computed when building the liveness information. I.e., I don't expect this to be a problem.
>
> > we allow a store of a physical register after the INLINEASM_BR when optimizations are disabled.
>
> That sounds wrong.
>
> Could we step back a little bit, what is the semantic of `INLINEASM_BR`?


[I'm sorry if some of this is basic. I just want to give a clear description of what's going on and why.]

INLINEASM_BR is an attempt to replicate the behavior of "asm goto". As such, it has one default destination (more-or-less the fallthrough block), and one or more indirect destinations. If the indirect destinations aren't taken, the resulting control flow is to fall out of the bottom of the ASM block. The callbr instruction is a terminator so that it can model this behavior as best as it can (which isn't all that great with LLVM's IR, but doable (see "invoke")). Once it's converted to INLINEASM_BR the behavior obviously doesn't change, but now that we have outputs, we need to figure out how to handle them given the constraints of MIR (i.e. you can't have non-terminals after a terminal, and live-ins are only expected to be correct after register allocation).

Because of how callbr is modeled, INLINEASM_BR is also a terminator. So during ISEL the values that are defined by INLINEASM_BR need to be spilled before jumping to the default block. Because of the constraint against having non-terminators after a terminator, we artificially modify the code after ISEL's finished building the block, placing any spills into a "copy" block (that's between the INLINEASM_BR and its default destination), and adding any registers defined by INLINEASM_BR as "live-ins" to the copy block. This is bad, because it violates the constraints, but was necessary at the time because we had no way to avoid it.

[Note that the outputs from an INLINEASM_BR are valid only on the default branch. It's very difficult to have them on the indirect branches without making the resulting code bad---both in style and in performance.]

Okay, so that's the behavior of INLINEASM_BR and some of the issues we came across that led us to here. I think TCOPY is a good solution because the default behavior of INLINEASM_BR is to fall out the bottom of the ASM block, so it would "naturally" execute the TCOPY instructions. And since we only ensure that the values are valid on the default branch, the COPYs will always be executed when needed (note a block containing an INLINEASM_BR should either exit to the default via a fallthrough or unconditional jump). It's still possible to split the block directly after the INLINEASM_BR and convert the TCOPYs into regular COPYs, in fact I would encourage us to do that at the appropriate point. It could remove a lot of the code in ScheduleDAGSDNodes.cpp.

> What happens if you put the copy at the beginning of each successor?

I found that this didn't work. CodeGen wants registers to be spilled at the end of a block. The spill slot is recorded and used later on when accessing the value. If we just add a COPY into the default block it won't have the necessary information available to load the correct value. We would have to add the information as a "live-in" on the default block, but that's a constraint violation, wash-rinse-repeat.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75098