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

[Daniel Berlin via llvm-commits]

(and as for whether to use po_iterator, yes, we don't have a good dfs
visitation scheme.
Unless you want to write better iterators with more control right now,  I
would just make a simple backedge discovery routine that uses a custom
worklist DFS.
GenericDomTreeConstruction.h has one you can copy, as does df_iterator.)




On Sat, Oct 1, 2016 at 2:05 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

> dberlin added inline comments.
>
>
> > david2050 wrote in ADCE.cpp:217
> > This seems super simple and conservative in the sense it will catch all
> loops. The variant of this code before this review cycle started used
> po_iterator but there no measurable benefit but quite a bit more code.
>
> So, here are two of identical code, laid out in the IR differently,  where
> we will do different things, and should not.
>
> A
> jump C
> B
> jump D
> C
> jump B
>
> A
> jump C
> C
> jump B
> B
> jump D
>
> In the first, the visitation order will be A, B, C.
> You will mark A as seen.
> No successors seen, nothing marked.
> You will now mark B as visited
> No successors seen, nothing marked
> You will now mark C as visited
> You will see that you have marked B as visited
> You will incorrectly think this is a back edge.
>
> In the second, the visitation order, will be A, C, B.
> You will mark A as seen.
> No successors seen, nothing marked.
> You will mark C as seen
> No successors seen, nothing marked.
> You will mark B as seen
> No successors seen, nothing marked
>
> > david2050 wrote in ADCE.cpp:228
> > Still can only mark the terminator of that block live once.
>
> yes, i read it wrong, sorry!
>
> > david2050 wrote in ADCE.cpp:248
> > I don't think so: marking the terminator of the source block not the
> successor block.
>
> yes, i read it wrong, sorry!
>
> https://reviews.llvm.org/D24918
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161001/f9f92884/attachment.html>