On Mon, Aug 13, 2012 at 12:57 PM, Richard Guenther
<richard.guent...@gmail.com> wrote:
> On Mon, Aug 13, 2012 at 12:22 PM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Mon, Aug 13, 2012 at 12:21 PM, Richard Guenther
>> <richard.guent...@gmail.com> wrote:
>>> On Sun, Aug 12, 2012 at 2:02 PM, Steven Bosscher <stevenb....@gmail.com>
>>> wrote:
>>>> On Sat, Aug 11, 2012 at 11:16 PM, Steven Bosscher <stevenb....@gmail.com>
>>>> wrote:
>>>>> Lots of test cases fail with the attached patch.
>>>>
>>>> Lots still fail after correcting the verifier :-)
>>>>
>>>> 920723-1.c: In function 'f':
>>>> 920723-1.c:14:1: error: bb 13 has loop depth 2, should be 1
>>>> f (int count, vector_t * pos, double r, double *rho)
>>>> ^
>>>> 920723-1.c:14:1: error: bb 14 has loop depth 2, should be 1
>>>> 920723-1.c:14:1: internal compiler error: in verify_loop_structure, at
>>>> cfgloop.c:1598
>>>
>>> That's a pre-existing bug in unswitching. When unswitching
>>> simplifies the condition it unswitches on using simplify_using_entry_checks
>>> it may turn an inner loop into an exit to an endless loop. But it does
>>> not modify the loop stucture according to this change.
>>>
>>> void foo (int x, int r)
>>> {
>>> loop4:
>>> if (r >= x)
>>> {
>>> goto loop4_latch;
>>> }
>>> else
>>> {
>>> loop5:
>>> if (r >= x)
>>> goto loop4_latch;
>>> goto loop5;
>>> }
>>> loop4_latch:
>>> goto loop4;
>>> }
>>>
>>> simplified testcase that even fails at -O1. We mostly rely on cfg-cleanup
>>> to fixup loops for us, so this is one case it does not handle properly.
>>
>> Actually that testcase fails verification right after a full loop
>> discovery which
>> DOM1 performs ...
>
> Fixed by attached patch.
>
>>> The quest of keeping loops up-to-date is hard ... but thanks for the
>>> checking
>>> code ;)
>
> Which probably still makes things fail elsewhere ;)
Same issue in fix_loop_structure:
/* Now fix the loop nesting. */
FOR_EACH_LOOP (li, loop, 0)
{
ploop = superloop[loop->num];
if (ploop != loop_outer (loop))
{
flow_loop_tree_node_remove (loop);
flow_loop_tree_node_add (ploop, loop);
}
}
I wonder why we cache loop-depth at all ... given that it is a "simple"
dereference bb->loop_father->superloops->base.prefix.num. For all
the hassle to keep that cache up-to-date, that is.
Would anybody mind removing basic_block->loop_depth? With C++
we can even have an overloaded loop_depth that works on both basic-blocks
and loops ...
Richard.
> Richard.
>
>>> Richard.
>>>
>>>> Ciao!
>>>> Steven
