[llvm-commits] [llvm] r168829 - /llvm/trunk/lib/Transforms/Scalar/CodeGenPrepare.cpp

[Nick Lewycky]

On 5 December 2012 16:29, Bill Wendling <isanbard at gmail.com> wrote:

> On Dec 5, 2012, at 4:23 PM, Nick Lewycky <nlewycky at google.com> wrote:
>
> > On 28 November 2012 15:23, Bill Wendling <isanbard at gmail.com> wrote:
> > Author: void
> > Date: Wed Nov 28 17:23:48 2012
> > New Revision: 168829
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=168829&view=rev
> > Log:
> > When we delete a dead basic block, see if any of its successors are dead
> and
> > delete those as well.
> >
> > Modified:
> >     llvm/trunk/lib/Transforms/Scalar/CodeGenPrepare.cpp
> >
> > Modified: llvm/trunk/lib/Transforms/Scalar/CodeGenPrepare.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/CodeGenPrepare.cpp?rev=168829&r1=168828&r2=168829&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Scalar/CodeGenPrepare.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Scalar/CodeGenPrepare.cpp Wed Nov 28
> 17:23:48 2012
> > @@ -194,9 +194,19 @@
> >            WorkList.insert(*II);
> >      }
> >
> > -    for (SmallPtrSet<BasicBlock*, 8>::iterator
> > -           I = WorkList.begin(), E = WorkList.end(); I != E; ++I)
> > -      DeleteDeadBlock(*I);
> > +    // Delete the dead blocks and any of their dead successors.
> > +    while (!WorkList.empty()) {
> > +      BasicBlock *BB = *WorkList.begin();
> > +      WorkList.erase(BB);
> > +      SmallVector<BasicBlock*, 2> Successors(succ_begin(BB),
> succ_end(BB));
> > +
> > +      DeleteDeadBlock(BB);
> >
> > I think you need to set MadeChange = true here. I think this bug could
> actually manifest because we use MadeChange later on to decide whether we
> need to recalculate the domtree.
> >
> Okay.
>
> > +
> > +      for (SmallVectorImpl<BasicBlock*>::iterator
> > +             II = Successors.begin(), IE = Successors.end(); II != IE;
> ++II)
> > +        if (pred_begin(*II) == pred_end(*II))
> >
> > What if the block includes a branch to itself?
> >
> Won't the predecessor list be non-empty then?
>

Right, and the block is dead and we want to delete it, but won't. This
isn't a bug, just a missed optz'n.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121205/a7efecdc/attachment.html>