Re: [PATCH PR68906]

[Yuri Rumyantsev <ysrumyan at gmail dot com>]

Richard,

Here is modified patch which checks only that exit block belongs to loop.

Bootstrapping and regression testing were successful.
Is it OK for trunk?

ChangeLog:
2014-12-17  Yuri Rumyantsev  <ysrumyan@gmail.com>

PR tree-optimization/68906
* tree-ssa-loop-unswitch.c (tree_unswitch_outer_loop): Add  a check
that an exit block belongs to LOOP.

gcc/testsuite/ChangeLog
* gcc.dg/torture/pr68906.c: New test.
.

2015-12-16 17:49 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Dec 16, 2015 at 3:36 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote:
>> Richard,
>>
>> Here is updated patch which includes (1) a test on exit proposed by
>> you and (2) another test from PR68021 which is caught by new check on
>> counted loop. Outer-loop unswitching is not performed for both new
>> tests.
>
> As said I don't think
>
>    /* If the loop is not expected to iterate, there is no need
>        for unswitching.  */
> -  iterations = estimated_loop_iterations_int (loop);
> -  if (iterations >= 0 && iterations <= 1)
> +  niters = number_of_latch_executions (loop);
> +  if (!niters || chrec_contains_undetermined (niters))
>      {
>
> is good.  We do want to retain the estimated_loop_iterations check
> as it takes into account profile data while yours lets through all
> counted loops.
>
> Also I don't see why SCEV needs to be able to analyze the IV to check
> for validity.
>
> Can you please split the patch into the part I suggested (which is ok)
> and the rest?
>
> Thanks,
> Richard.
>
>>
>> Bootstrapping and regression testing did not show any new failures.
>>
>> Is it OK for trunk.
>>
>> ChangeLog:
>> 2014-12-16  Yuri Rumyantsev  <ysrumyan@gmail.com>
>>
>> PR tree-optimization/68021
>> PR tree-optimization/68906
>> * tree-ssa-loop-unswitch.c : Include couple header files.
>> (tree_unswitch_outer_loop): Add check that an exit is not inside inner
>> loop, use number_of_latch_executions to detect non-iterated loops.
>>
>> gcc/testsuite/ChangeLog
>> * gcc.dg/torture/pr68021.c: New test.
>> * gcc.dg/torture/pr68906.c: Likewise.
>>
>> 2015-12-16 15:51 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>>> On Wed, Dec 16, 2015 at 1:14 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote:
>>>> Hi All,
>>>>
>>>> Here is simple patch which cures the issue with outer-loop unswitching
>>>> - added invocation of number_of_latch_executions() to reject
>>>> unswitching for non-iterated loops.
>>>>
>>>> Bootstrapping and regression testing did not show any new failures.
>>>> Is it OK for trunk?
>>>
>>> No, that looks like just papering over the issue.
>>>
>>> The issue (with the 2nd testcase at least) is that single_exit () accepts
>>> an exit from the inner loop.
>>>
>>> Index: gcc/tree-ssa-loop-unswitch.c
>>> ===================================================================
>>> --- gcc/tree-ssa-loop-unswitch.c        (revision 231686)
>>> +++ gcc/tree-ssa-loop-unswitch.c        (working copy)
>>> @@ -431,7 +431,7 @@ tree_unswitch_outer_loop (struct loop *l
>>>      return false;
>>>    /* Accept loops with single exit only.  */
>>>    exit = single_exit (loop);
>>> -  if (!exit)
>>> +  if (!exit || exit->src->loop_father != loop)
>>>      return false;
>>>    /* Check that phi argument of exit edge is not defined inside loop.  */
>>>    if (!check_exit_phi (loop))
>>>
>>> fixes the runtime testcase for me (not suitable for the testsuite due
>>> to the infinite
>>> looping though).
>>>
>>> Can you please bootstrap/test the above with your testcase?  The above patch is
>>> ok if it passes testing (no time myself right now)
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> ChangeLog:
>>>>
>>>> 2014-12-16  Yuri Rumyantsev  <ysrumyan@gmail.com>
>>>>
>>>> PR tree-optimization/68906
>>>> * tree-ssa-loop-unswitch.c : Include couple header files.
>>>> (tree_unswitch_outer_loop): Use number_of_latch_executions
>>>> to reject non-iterated loops.
>>>>
>>>> gcc/testsuite/ChangeLog
>>>> * gcc.dg/torture/pr68906.c: New test.

Attachment: patch.2
Description: Binary data