[PATCH] D88823: [CodeGen][TailDuplicator] Don't duplicate blocks with INLINEASM_BR

Mon Oct 5 19:52:57 PDT 2020

void added a comment.

In D88823#2313262 <https://reviews.llvm.org/D88823#2313262>, @nickdesaulniers wrote:

>>> %9:gr32 = PHI %0:gr32, %bb.1, %5:gr32, %bb.2
>>
>> The problem is this PHI here. We don't have a way to resolve PHI nodes in the entry block to an indirect destination. (This is the reason we don't allow "asm goto" outputs on the indirect paths.) When we go to resolve this PHI node, there's no place to put the put the values that works in all situations; we can't split the critical edges and it's not valid to place the resolved instructions at the ends of the predecessor blocks.
>
> We're also not using "asm goto" outputs in this test case, so I still don't understand why this PHI is problematic.

That doesn't matter. It's problematic because of the PHI in the block with the INLINEASM_BR. As I mentioned, the result of the tail duplication is that that PHI is more-or-less moved *down* into the indirect and default destinations. Look at those PHIs and you'll see that they have the same values coming into them.

>> We don't have a way to resolve PHI nodes in the entry block to an indirect destination.
>
> We should be able to catch cases where we have a `MachineBasicBlock` whose address is taken, yet starts with a `phi`, right?  Then an assert or `-verify-machineinstrs` check should help avoid producing such cases.

That's not what's wrong. A PHI is fine in the default branch. It shouldn't exist in the indirect branch, because *the back end can't currently handle it*. And even if it could, the semantics of the asm goto just don't allow for a PHI to exist in the indirect branch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88823