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

[Zoltán Majó]

Hi Doug,


On 02/15/2016 04:44 PM, Doug Simon wrote:
> 1460     // cases, so give the attempt to unroll the loop (but still continue
> 1461     // compilation).
>
> is that meant to be “so give up on the attempt”?

yes, that is what I meant.

This is what happens when re-editing the text and forgetting to re-read 
the complete sentence. Thank you for catching the mistake!

Here is the updated webrev:
http://cr.openjdk.java.net/~zmajo/8148754/webrev.01/

Best regards,


Zoltan
>
>> On 15 Feb 2016, at 16:22, Zoltán Majó <zoltan.majo at oracle.com> 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
>>