[PATCH] D20907: [IfConversion] Bugfix: don't add Undef flag on use if reg is live.

Jonas Paulsson via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 1 06:42:45 PDT 2016


jonpa added a comment.

> The IfConverter currently does those fixups which should be good enough to model the liveness correctly (in a conservative manner). I still consider this somewhat a hack as the relations between defs and uses are bogus after this conversion, we are merely saved by the fact that the IfConversion pass runs really late and we do not have any later passes looking into def/use relationships.


With this patch, it should be conservatively correct, as you say. It would also be possible to teach buildSchedGraph() to skip adding edges where two different predicates are known to be disjoint, i.e. for two defs of the same reg.

> To fix calls clobbering a live register; "pseudo" live in the sense that it is only live in our "ignore the predicates" worldview. I believe we have to add a implicit def and(!) and implicit use in the presence of clobbers or dead definitions.


I am not following why predicating a call with a regmask would require adding any extra operands at all - do you have an example?

> I believe that r275201 resolved the problems here and you should try pushing this again Jonas!


Great about the live-in lists :-)

Still seem to me though, that IfConversion should be fixed to have the right liveness tracking. According to earlier discussion, it is just not true that a reg that is live into one successor is live into both successors, and therefore I still think these lines needs fixing:

  // Initialize liveins to the first BB. These are potentially redefined by
  // predicated instructions.
  Redefs.init(TRI);
  Redefs.addLiveIns(*CvtBBI->BB);
  Redefs.addLiveIns(*NextBBI->BB);

(As Quentin pointed out, this is conservatively correct, but the MachineVerifier complains, because the IfConversion pass may add an implicit-use op on an instruction in one of the successors, while it was not actually live into that MBB, but the other)


https://reviews.llvm.org/D20907





More information about the llvm-commits mailing list