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

[Daniel Berlin via llvm-commits]

(all that said, if you want to approve it as-is, i'm not going to object)

On Thu, Nov 17, 2016 at 7:46 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
>
> 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/97f929fd/attachment.html>