[PATCH] D29620: [JumpThreading] Thread through guards
Sanjoy Das via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 15 09:36:33 PST 2017
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)
----------------
sanjoy wrote:
> 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`.
Also, you would not have had two preds for `BB`.
Repository:
rL LLVM
https://reviews.llvm.org/D29620
More information about the llvm-commits
mailing list