[PATCH] D29620: [JumpThreading] Thread through guards

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp:2076
+    if (Parent == Pred2->getSinglePredecessor())
+      if (BranchInst *BI = dyn_cast<BranchInst>(Parent->getTerminator()))
+        for (auto &I : *BB)
----------------
mkazantsev wrote:
> sanjoy wrote:
> > mkazantsev wrote:
> > > anna wrote:
> > > > Hi Max, I think you need to check that branch is a conditional branch. Otherwise we can fail while trying to get the condition for the branch in `ThreadGuard` at line 2091.
> > > You're right, I need to add this check here.
> > I guess this is possible when `Pred1` and `Pred2` are the same, like:
> > 
> > 
> > ```
> > Parent:
> >   br label %pred
> > 
> > pred:
> >   switch ... 4 -> BB; 5 -> BB; ...
> > 
> > BB:
> >   ...
> > ```
> > 
> > 
> > Are there other bits in this patch that are wrong in that situation?
> > 
> I think another example could be
> 
>   Parent:
>     br %cond Pred, Pred
> 
>   Pred:
>     br BB
> 
>   BB
>     ...
> 
> So I would rather add both checks "branch is conditional" and pred1 != pred2.
I think `getSinglePredecessor` on `Pred` will return `nullptr` in that case.  `getUniquePredecessor` would have returned `Parent`.


Repository:
  rL LLVM

https://reviews.llvm.org/D29620