Improve DOM's optimization of control statements

On 10/02/2015 05:15 AM, Renlin Li wrote:
> Hi Jeff,
>
> Your patch causes an ICE regression.
> The test case is " gcc.c-torture/compile/pr27087.c", I observed it on
> aarch64-none-elf target when compiling the test case with '-Os' flag.
>
> A quick check shows, the cfg has been changed, but the loop information
> is not updated. Thus the information about the number of basic block in
> a loop is not reliable.
>
> Could you please have a look?
As I mentioned, when we collapse a conditional inside a loop, we may 
change the # of nodes in a loop which edges are exit edges and possibly 
other stuff.  So we need to mark loops as needing fixups.

Verified this fixes the aarch64-elf regression and did a bootstrap & 
regression test on x86_64-linux-gnu.

Installed on the trunk.

jeff
commit 992d281b2d1ba53a49198db44fee92a505e16f5d
Author: Jeff Law <law@tor.usersys.redhat.com>
Date:   Fri Oct 2 15:22:04 2015 -0400

    Re: [PATCH] Improve DOM's optimization of control statements
    
    	* tree-ssa-dom.c (optimize_stmt): Note when loop structures need
    	fixups.

On Fri, Oct 2, 2015 at 9:30 PM, Jeff Law <law@redhat.com> wrote:
> On 10/02/2015 05:15 AM, Renlin Li wrote:
>>
>> Hi Jeff,
>>
>> Your patch causes an ICE regression.
>> The test case is " gcc.c-torture/compile/pr27087.c", I observed it on
>> aarch64-none-elf target when compiling the test case with '-Os' flag.
>>
>> A quick check shows, the cfg has been changed, but the loop information
>> is not updated. Thus the information about the number of basic block in
>> a loop is not reliable.
>>
>> Could you please have a look?
>
> As I mentioned, when we collapse a conditional inside a loop, we may change
> the # of nodes in a loop which edges are exit edges and possibly other
> stuff.  So we need to mark loops as needing fixups.
>
> Verified this fixes the aarch64-elf regression and did a bootstrap &
> regression test on x86_64-linux-gnu.
>
> Installed on the trunk.
>
> jeff
>
> commit 992d281b2d1ba53a49198db44fee92a505e16f5d
> Author: Jeff Law <law@tor.usersys.redhat.com>
> Date:   Fri Oct 2 15:22:04 2015 -0400
>
>     Re: [PATCH] Improve DOM's optimization of control statements
>
>         * tree-ssa-dom.c (optimize_stmt): Note when loop structures need
>         fixups.
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 3f7561a..e541df3 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-10-02  Jeff Law  <law@redhat.com>
> +
> +       * tree-ssa-dom.c (optimize_stmt): Note when loop structures need
> +       fixups.
> +
>  2015-10-02  Uros Bizjak  <ubizjak@gmail.com>
>
>         * system.h (ROUND_UP): New macro definition.
> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
> index a8b7038..d940816 100644
> --- a/gcc/tree-ssa-dom.c
> +++ b/gcc/tree-ssa-dom.c
> @@ -1843,6 +1843,12 @@ optimize_stmt (basic_block bb, gimple_stmt_iterator
> si,
>               /* Delete threads that start at BB.  */
>               remove_jump_threads_starting_at (bb);
>
> +             /* If BB is in a loop, then removing an outgoing edge from BB
> +                may cause BB to move outside the loop, changes in the
> +                loop exit edges, etc.  So note that loops need fixing.  */
> +             if (bb_loop_depth (bb) > 0)
> +               loops_state_set (LOOPS_NEED_FIXUP);
> +

I would rather do this in remove_ctrl_stmt_and_useless_edges and only
if taken_edge is a loop exit.  loop fixup is a pretty big hammer which
we should avoid at all cost.

So please try to be more specific on the cases you invoke it.

Thanks,
Richard.

>               /* Now clean up the control statement at the end of
>                  BB and remove unexecutable edges.  */
>               remove_ctrl_stmt_and_useless_edges (bb, taken_edge->dest);
>

On 10/05/2015 03:02 AM, Richard Biener wrote:
>> +             /* If BB is in a loop, then removing an outgoing edge from BB
>> +                may cause BB to move outside the loop, changes in the
>> +                loop exit edges, etc.  So note that loops need fixing.  */
>> +             if (bb_loop_depth (bb) > 0)
>> +               loops_state_set (LOOPS_NEED_FIXUP);
>> +
>
> I would rather do this in remove_ctrl_stmt_and_useless_edges and only
> if taken_edge is a loop exit.  loop fixup is a pretty big hammer which
> we should avoid at all cost.
remove_ctrl_stmt_and_useless_edges is used internally when duplicating 
blocks for jump threading, so we'd need to audit for that case as well.

>
> So please try to be more specific on the cases you invoke it.
I'd pondered this and was still working through whether or not 
triggering only when one of the outgoing edges was "interesting" from a 
loop structure standpoint.  I hadn't tested that yet and wanted to get 
the regression resolved, hence the large hammer.


jeff