[PATCH] D68267: [MBB LiveIn lists, MachineVerifier, SystemZ] New method isLiveOut() and mverifier improvement.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:472
 
+bool MachineBasicBlock::isLiveOut(MCRegister PhysReg) const {
+  assert(PhysReg.isPhysical() && "Expected physreg");
----------------
jonpa wrote:
> bjope wrote:
> > bjope wrote:
> > > Afaict this does not work for callee saved registers (nor pristine regs (not used in the entire function), nor when being live out from the exit block). Nor does it work for reserved registers (aren't they always considered live out from the exit block?).
> > > 
> > Well, the description of the function in the header file specifies the condition for the check, so it works as specified. But given the name I suppose it could be an easy mistake to believe that it more general. But maybe isLiveIn() also is limited to a subset of all registers (?), and then this isn't making anything any worse.
> Thanks for taking a look :-)
> 
> I was aware that pristine and reserved registers would be excepted from live-in lists, but I hadn't thought about callee saved registers - why would they be excluded? And I think liveness is (in this context) a per function problem, so I don't see that the exit block case is part of it, right?
> 
> I would be open to a name change and perhaps an assert against calling this on a pristine/reserved register. But as you said, the specification in the comment is quite clear so it is up to the user determine when this is useful given that information.
> 
> 
I'm not really saying that those registers should be excluded. My thought were about the result in case this method was called on an exit block. Exit blocks do not have any successors, but they might have lots of live out registers. So only looking at the successor list seemed incomplete. I simply listed registers that are live out from the exit blocks, but currently not being reported as live out.


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

https://reviews.llvm.org/D68267