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

[Jeff Law <law at redhat dot com>]

On 03/19/2015 03:16 AM, Richard Biener wrote:
> 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.
Right.  The assumption of orderings was the initial thing that jumped 
out at me.  While it may be the case that in practice we're going to be 
presented with blocks in that kind of order, depending on it seems unwise.

Jeff