Re: Gimple loop splitting

[Jeff Law <law at redhat dot com>]

On 11/16/2015 09:05 AM, Michael Matz wrote:
> It's a prerequisite indeed, but not enough in itself.
Sigh.  OK.  One can always hope.



> hmmer is actually quite interesting because it's a fairly isolated hot
> loop posing quite challenging problems for us :)
Sounds like it.  Essentially, it's a TODO list :-)

> > Probably ought to be disabled when we're not optimizing for speed as well.
>
> That should be dealt with by '!optimize_loop_for_size_p (loop)'.
Doh, must have missed that.

> > Please clean up the #if 0/#if 1 code in the new tests.
>
> Actually I'd prefer if that test contains the by-hand code and the TRACE
> stuff as well, I'd only change the #if 0 into some #if BYHAND or so ...
>
> > You might also want to clean out the TRACE stuff.  Essentially the tests
> > look like you just dropped in a test you'd been running by hand until
> > now :-)
>
> ... the reason being, that bugs in the splitter are somewhat unwieldy to
> debug by just staring at the dumps, you only get a checksum mismatch, so
> TRACE=1 is for finding out which of the params and loops is actually
> miscompiled, TRACE=2 for finding the specific iteration that's broken, and
> the #if0 code for putting that situation into a non-macroized and smaller
> function than dotest.  (That's actually how I've run the testcase after I
> had it basically working, extending dotest with a couple more lines, aka
> example loop sitations, adjusting the checksum, and then making a face and
> scratching my head and mucking with the TRACE and #if0 macros :) ).
OK, if you want to keep them, then  have a consistent way to turn them 
on/off for future debugging.  if0/if1 doesn't provide much of a clue to 
someone else what to turn on/off if they need to debug this stuff.

> > I don't see any negative tests -- ie tests that should not be split due
> > to boundary conditions.  Do you have any from development?
>
> Good point, I had some but only ones where I was able to extend the
> splitters to cover them.  I'll think of some that really shouldn't be
> split.
If you've got them, certainly add them.  Though I realize they may get 
lost over time.

> Only with such situations:
>
>    for (int i = start; i < end; i++) {
>      if (i + offset < bound)
>        ...
>    }
>
> Here the condition-IV is not directly defined by a PHI node.  If it
> happens often I don't know, I guess the usual situation is testing the
> control IV directly.  The deficiency is not hard to fix.
I'm comfortable waiting until we see the need.

> I think I convinced myself on paper that the dominator tree is correct due
> to our helpers doing the right thing (loop_version() for the initial
> loop copying and split_edge for the above diddling).  Let's see if I can
> paint some ASCII art.  So, after loop_version (which updates dom) we
> have:
OK.  I was worried about the next step -- where we insert the 
conditional on the exit from pre1 to have it transfer to join or pre2.

But in that case, the immediate dominator of pre2 & join is still the 
initial if statement.  So I think we're OK.  That was the conclusion I 
was starting to come to yesterday, having the ascii art makes it pretty 
clear.  I'm just not good at conceptualizing a CFG.  I have to see it 
explicitly and then everything seems so clear and simple.

jeff