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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa marked 3 inline comments as done.
jonpa added inline comments.


================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:472
 
+bool MachineBasicBlock::isLiveOut(MCRegister PhysReg) const {
+  assert(PhysReg.isPhysical() && "Expected physreg");
----------------
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.




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

https://reviews.llvm.org/D68267