[PATCH] D46568: [SimplifyCFG] Fix a crash when folding PHIs

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:152
   // Begin by getting rid of unneeded PHIs.
   SmallVector<Value *, 4> IncomingValues;
   if (isa<PHINode>(BB->front())) {
----------------
AssertingVH?  (This would also make it easier to reduce the testcase, by turning undefined behavior into a simple assertion failure.)


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:155
     for (PHINode &PN : BB->phis())
-      if (PN.getIncomingValue(0) != &PN)
+      if (!isa<PHINode>(PN.getIncomingValue(0)))
         IncomingValues.push_back(PN.getIncomingValue(0));
----------------
vsk wrote:
> Would it be safe/worthwhile to allow incoming values which are phi nodes from a block other than `BB`? These should not be invalidated by FoldSingleEntryPHINodes(). If the goal is to eliminate duplicate dbg.values for incoming values, then it might make sense.
> 
> Apologies if this is off-base, I'm not familiar with this code.
Maybe `!isa<PHINode>(PN.getIncomingValue(0)) || cast<PHINode>(PN.getIncomingValue(0))->getParent() != BB`, or something like that, so we preserve debug info in more cases?


https://reviews.llvm.org/D46568