[9] RFR (S): 8148754: C2 loop unrolling fails due to unexpected graph shape

[Vladimir Kozlov]

Zoltan,

It should not be "main" loop if peeling happened. See do_peeling():

     if (cl->is_main_loop()) {
       cl->set_normal_loop();

Split-if optimization should not split through loop's phi. And generally 
not through loop's head since it is not making code better - split 
through backedge moves code into loop again. Making loop body more 
complicated as this case shows.

Bailout unrolling is fine but performance may suffer because in some 
cases loop unrolling is better then split-if.

Thanks,
Vladimir

On 2/15/16 7:22 AM, Zoltán Majó wrote:
> Hi,
>
>
> please review the patch for 8148754.
>
> https://bugs.openjdk.java.net/browse/JDK-8148754
>
> Problem: Compilation fails when the C2 compiler attempts loop unrolling.
> The cause of the failure is that the loop unrolling optimization expects
> a well-defined graph shape at the entry control of a 'CountedLoopNode'
> ('IfTrue'/'IfFalse' preceeded by 'If' preceeded by 'Bool' preceeded by
> 'CmpI').
>
>
> Solution: I investigated several different instances of the same
> failure. It turns out that the shape of the graph at a loop's entry
> control is often different from the way loop unrolling expects it to be
> (please find some examples in the bug's JBS issue). The various graph
> shapes are a result of previously performed transformations, e.g.,
> split-if optimization and loop peeling.
>
> Loop unrolling requires the above mentioned graph shape so that it can
> adjust the zero-trip guard of the loop. With the unexpected graph
> shapes, it is not possible to perform loop unrolling. However, the graph
> is still in a valid state (except for loop unrolling) and can be used to
> produce correct code.
>
> I propose that (1) we check if an unexpected graph shape is encountered
> and (2) bail out of loop unrolling if it is (but not fail in the
> compiler in such cases).
>
> The failure was triggered by Aleksey's Indify String Concatenation
> changes but the generated bytecodes are valid. So this seems to be a
> compiler issue that was previously there but was not yet triggered.
>
>
> Webrev:
> http://cr.openjdk.java.net/~zmajo/8148754/webrev.00/
>
> Testing:
> - JPRT;
> - local testing (linux-86_64) with the failing test case;
> - executed all hotspot tests locally, all tests pass that pass with an
> unmodified build.
>
> Thank you!
>
> Best regards,
>
>
> Zoltan
>