[PATCH] D24918: [ADCE] Add code to remove dead branches

[Daniel Berlin via llvm-commits]

On Wed, Nov 16, 2016 at 9:57 PM, Mehdi AMINI <mehdi.amini at apple.com> wrote:

> mehdi_amini added a comment.
>
> Sorry for the delay, I keep getting interrupted in the middle of getting
> this code into my head, and paging out has a real cost when getting back to
> it. Especially as I don't really know the algorithm, I have to infer what
> you're doing from the implementation (which isn't a bad way to judge of the
> clarity of the code).
>
> Anyway, a few inline comments.
>
>
>
> ================
> Comment at: lib/Transforms/Scalar/ADCE.cpp:610
> +  }
> +
> +  // Update each live phi based on the new incoming edges.
> ----------------
> david2050 wrote:
> > dberlin wrote:
> > > Is the above really necessary?
> > >
> > > The standard way to do this, AFAIK, is to mark the useful block set
> while doing marking, and then just walking up the PDT to find the nearest
> block marked useful for each dead branch.
> > > Replace all uses of the bb with that block using RAUW.
> > > This should update phi nodes, since I believe the blocks in phi nodes
> are still considered uses.
> > >
> > I don't understand this approach. Given a live branch  (x,y) where y is
> dead and  z is the live post-dominator of y, we can't just replace uses of
> 'y' because the 'y' may not be directly referenced in phi nodes in 'z'
> which only will old references to its predecessors. Also, 'y' could have
> multiple branches into it which are dead.
> I'm not sure I understood what @dberlin referred to,


I am referring to pretty much what every compiler optimization class and
textbook list as the way to do this :)


> but as I read it it should be possible to just process DeadBlocks without
> computing the dead region, and only manipulate Phi in the live PDOM if the
> live PDOM is a direct successor of the current block. I think this should
> get the same result in term of Phi, but does not seems less costly than
> computing the region.
>

In most compilers, it's like5-10 lines of code, so while maybe not less
expensive, it's less complex.



> Also I'm not sure if you can rewrite the branches from the live
> predecessors into the dead region to jump directly to the live PDOM without
> computing the region (but with the invariant you are already relying on to
> get a dead region with a unique live PDOM it may be possible).
>

I linked both the papers and the implementation, but, like i said, this is
the standard method.
Keith Cooper's comp 512 slides also go over it
https://www.clear.rice.edu/comp512/Lectures/10Dead-Clean-SCCP.pdf

I believe it is also gone over in "*Engineering a Compiler, 2nd Edition
<http://store.elsevier.com/product.jsp?isbn=9780120884780>"
<http://store.elsevier.com/product.jsp?isbn=9780120884780>*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161117/939ad597/attachment.html>