[PATCH] D68128: [InstCombine] Fold PHIs with equal incoming pointers

[Daniil Suchkov via Phabricator via llvm-commits]

DaniilSuchkov marked an inline comment as done.
DaniilSuchkov added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:1163-1164
+      // Make sure the insertion point exists.
+      if (InsertPtIter != InsertBB->end())
+        InsertPt = &*InsertPtIter;
+    } else
----------------
apilipenko wrote:
> It's ok to insert at the end of BB. 
> 
> Use `IRBuilder::SetInsertPoint(BasicBlock *TheBB, BasicBlock::iterator IP)` for that. After this change there will be no situation when `InsertPt` is available, so the code can structured slightly differently:
> ```
>   if (auto *BaseInst = dyn_cast<Instruction>(Base)) {
>     if (isa<PHINode>(BaseInst))
>       Builder.SetInsertPoint(BaseInst->getParent(), BaseInst->getParent()->getFirstInsertionPt())
>     else
>       Builder.SetInsertPoint(BaseInst->getNextNode());
>   } else
>     Builder.SetInsertPoint(...);
> ``` 
> It's ok to insert at the end of BB.

It can result in broken IR by insertion of a regular instruction after terminator.

Take a look at "test_neg_gep_and_bitcast_phi_no_insert_pt", in that case there is no valid insertion point in bb at all:
```
catch:
  ; There is no insertion point in this basic block!
  %obj = phi i8* [ %obj1, %entry ], [ %obj2, %cont ]
  %cs = catchswitch within none [label %doit] unwind to caller
```


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

https://reviews.llvm.org/D68128