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

Brian Rzycki via Phabricator via llvm-commits llvm-commits at lists.llvm.org
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





More information about the llvm-commits mailing list