On Thu, Dec 17, 2015 at 10:21 AM, Yuri Rumyantsev <ysrum...@gmail.com> wrote:
> 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?

Ok with the spurious

@@ -441,7 +441,7 @@
   iterations = estimated_loop_iterations_int (loop);
   if (iterations >= 0 && iterations <= 1)
     {
-      if (dump_file && (dump_flags & TDF_DETAILS))
+      if (dump_enabled_p ())
        fprintf (dump_file, ";; Not unswitching, loop is not expected"
                 " to iterate\n");
       return false;

hunk removed.

Thanks,
Richard.

> ChangeLog:
> 2014-12-17  Yuri Rumyantsev  <ysrum...@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.guent...@gmail.com>:
>> On Wed, Dec 16, 2015 at 3:36 PM, Yuri Rumyantsev <ysrum...@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  <ysrum...@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.guent...@gmail.com>:
>>>> On Wed, Dec 16, 2015 at 1:14 PM, Yuri Rumyantsev <ysrum...@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  <ysrum...@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.

Reply via email to