[PATCH] D61576: [LoopInterchange] Fix handling of LCSSA nodes defined in headers and latches.

Mon May 6 05:00:43 PDT 2019

fhahn marked 2 inline comments as done.
fhahn added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:1331
+    assert(P.hasOneUse() &&
+           "Can only deal with PHI nodes with single users here.");
+    PHINode *UP = dyn_cast<PHINode>(*P.user_begin());
----------------
jdoerfert wrote:
> This seems brittle. I don't know all the preconditions here but I imagine various situations where you might end up with multiple uses for escaping LCSSA phi nodes. 
> 
> One might be two values escape which happen to be equal, in the inner exit block some optimization merges the two LCSSA phi nodes, in the outer that doesn't happen. Now we have one PHI in the inner exit and two uses in the outer exists. Does this make sense (without asserting we have a transformation like this right now)?
Yeah, it could theoretically happen (but I think it is unlikely in practice, as the lcssa PHIs would be trivially equivalent). The single use is actually not important. It is important where those uses actually are. The legality checks should ensure that. I updated the assertion.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61576