Re: Fix PR 65177: diamonds are not valid execution threads for jump threading

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Mar 18, 2015 at 11:35 PM, Sebastian Pop <sebpop@gmail.com> wrote:
> Hi,
>
> the attached patch fixes PR 65177 in which the code generator of FSM jump thread
> would create a diamond on the copied path: see https://gcc.gnu.org/PR65177#c18
> for a detailed description.
>
> The patch is renaming SEME into jump_thread as the notion of SEME is more
> general than the special case that we are interested in, in the case of
> jump-threading: an execution thread to be jump-threaded has the property that
> each node on the path has exactly one predecessor, disallowing any other
> control flow like diamonds or back-edge loops within the SEME region.
>
> The patch passes regstrap on x86-64-linux, and fixes the make check of hmmer.
> Ok to commit?

I don't like the special-casing in copy_bbs (with bb_in_bbs doing a
linear walk anyway...).
Is the first test

+             /* When creating a jump-thread, we only redirect edges to
+                consecutive basic blocks.  */
+             if (i + 1 < n)
+               {
+                 if (e->dest != bbs[i + 1])
+                   continue;

not really always the case for jump threads?  copy_bbs doesn't impose
any restriction
on ordering on bbs[], so it seems to be a speciality of the caller.

+               {
+                 /* Do not jump back into the jump-thread path from the last
+                    BB.  */
+                 if (bb_in_bbs (e->dest, bbs, n - 1))
+                   continue;

this one sounds easier to ensure in the caller as well - just remove the extra
unwanted edge.

That said - please instead fixup after copy_bbs in duplicate_seme_region.

Richard.

> Thanks,
> Sebastian