[PATCH] D79794: Change the INLINEASM_BR MachineInstr to be a non-terminating instruction.

Bill Wendling via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 4 13:16:25 PDT 2021


void added a comment.

In D79794#3040927 <https://reviews.llvm.org/D79794#3040927>, @MatzeB wrote:

>> We took great pains to ensure that nothing could be moved past it, so that the semantics won't change and we can reload registers, etc..
>
> Yes, I see that this is going through great pain to ensure things still work. But this all feels to me like you are making a form of EBBs work here. There is a way to jump out of the basic block without having all instructions in it executed. There must be some cases where COPYs or SPILLs insructions end up after the branch now which brings us into this trouble.

This is a general issue with "asm goto", not just with "asm goto with outputs". Someone could stomp on registers, memory, and the like and leap to BFE without the necessary COPY / SPILL instructions to clean things up. We skirt around some of these issues by saying that outputs aren't valid on the indirect branch.

There's a reason why "asm goto" wasn't implemented in LLVM until we were forced to do it...

> - I realize that this was painful to model since LLVM was never prepared to have terminator instructions produce values. I guess we lack the time+expertise to fix this after the fact now :-(
> - The design was already messed up before `INLINEASM_BR` for exception handling.
>
> So I don't know what to do here except rant and predict random bugs popping up over the years because intuition about how basic blocks work is broken.

One method to perhaps address your concerns would be to create a "terminating" COPY instruction. This allows one to reload registers after the ASM block. There were some sticky issues with it. Going this method was much easier. (Yes, I know that's not the best reason to do something, but...) However, the only issue a hypothetical terminating COPY would solve is issues with instructions invalidly moving past the `INLINEASM_BR`.

I would *love* to fix this. Chris claims that MLIR would solve all of this. (nudge...nudge) :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79794



More information about the llvm-commits mailing list