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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert 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());
----------------
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)?


================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:1335
+    (void)UP;
+    P.use_begin()->set(P.getIncomingValue(0));
+    P.eraseFromParent();
----------------
Isn't this a job for `P.replaceAllUsesWith(...)` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61576