[PATCH] D63913: [JumpThreading] Fix threading with unusual PHI nodes.

Fri Jun 28 09:22:10 PDT 2019

brzycki added a comment.

Hi @efriedma , thank you for debugging and identifying the true root cause of PR42085 <https://bugs.llvm.org/show_bug.cgi?id=42085>. My comments are inline.

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:110
+
+
 namespace {
----------------
I have concerns about adding this flag in this patch without a bit more testing and general test-cases for loop header threading. I fear this is a proverbial can of worms for any third party user of `opt` who discovers this flag and starts using it.

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1993
+  for (; PHINode *PN = dyn_cast<PHINode>(BI); ++BI) {
+    PHINode *New = PHINode::Create(PN->getType(), 1, PN->getName(), NewBB);
+    New->addIncoming(PN->getIncomingValueForBlock(PredBB), PredBB);
----------------
Nit: please consider naming `New` to something like `NewPN`.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63913