[PATCH] D79055: [LiveVariables] Mark PhysReg implicit-def MachineOperands of INLINEASM_BR as LiveOut

[Bill Wendling via Phabricator via llvm-commits]

void added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveVariables.cpp:24
 // register is not register allocatable, it is not tracked.  This is useful for
 // things like the stack pointer and condition codes.
 //
----------------
efriedma wrote:
> I'm concerned about the "allowing it to do a single local analysis to resolve physical register lifetimes in each basic block" part.  Is this change enough to actually make the liveness reliable?  In particular, I'm concerned what might happens if the fallthrough successor of an INLINEASM_BR has other predecessors.  (Or is that impossible?)
Drive-by comment:

INLINEASM_BR's fallthrough successor can have multiple predecessors, at least in theory (I don't believe there's a validation check preventing it). E.g.:

```
a(bool b) {
  if (b)
    asm goto("" : : : : indirect1);
  else
    asm goto("" : : : : indirect2);
  // Both "asm goto"s fallthrough to here.
indirect1:
...
indirect2:
...
}
```

In the IR, I believe clang generates a fallthrough block for each "asm goto" even though they both end up falling through to the same place. But because of general transformations, those can be elided of course, especially when converted to machine instructions.

My original implementation created a separate "copy" MBB that has only one predecessor---the block with the INLINEASM_BR instruction. It acts as a block that would be created during critical edge splitting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79055