Re: [PATCH] Don't bypass blocks with multiple latch edges (PR middle-end/54838)

[Eric Botcazou <ebotcazou@adacore.com>]

> RTL cprop seems to run both before and after RTL loop optimizers (currently
> after RTL loop optimizers we throw away loops - an arbitrary chosen point
> before IRA across which I could not get things to work).  Thus you could do
> 
>   if (current_loops)
>     is_loop_header = bb == bb->loop_father->header;
>   else
>     {
>   may_be_loop_header = false;
>   FOR_EACH_EDGE (e, ei, bb->preds)
>     if (e->flags & EDGE_DFS_BACK)
>       {
>         may_be_loop_header = true;
>         break;
>       }
>     }

OK, let's do something like this then:

  if (current_loops)
    may_be_loop_header = (bb == bb->loop_father->header);
  else
    {
       may_be_loop_header = false;
       FOR_EACH_EDGE (e, ei, bb->preds)
       if (e->flags & EDGE_DFS_BACK)
         {
          may_be_loop_header = true;
          break;
         }
    }

since we cannot always be sure that it's a header.

> I don't understand
> 
>       /* The irreducible loops created by redirecting of edges entering the
>          loop from outside would decrease effectiveness of some of the
>          following optimizations, so prevent this.  */
>       if (may_be_loop_header
>           && !(e->flags & EDGE_DFS_BACK))
>         {
>           ei_next (&ei);
>           continue;
>         }
> 
> why isn't this simply
> 
>       if (may_be_loop_header)
>         {
>           ei_next (&ei);
>           continue;
>         }
> 
> ?  It looks like the code tries to allow "rotating" a loop - but that's only
> good if bb has exactly two predecessors (one entry and one latch edge). And
> even then it requires to manually update the loop structures (update what
> the new header and latch blocks are).

That's the bug.  We have a loop with 2 latch edges, but the loop fixup code in 
bypass_block only handles one.

> That said, removing the !(e->flags & EDGE_DFS_BACK) condition seems
> to fix the ICE.  Threading across a loop header is in fact complicated
> (see the special routine tree-ssa-threadupdate.c:thread_through_loop_header
> necessary for that).

Scary enough indeed.  But this seems to work fine here if the loop has exactly 
one latch edge, so we don't need to change that.  Removing the above condition 
is equivalent to early returning from bypass_block for all potential headers.

> Let's declare the GIMPLE level did all interesting threadings through
> headers.

The testcase is precisely a counterexample with 2 latch edges.

But OK, let's punt if there is more than a single (potential) latch edge.

-- 
Eric Botcazou