RFR(M): 8229495: SIGILL in C2 generated OSR compilation

[Patric Hedlin]

Dear all,

I would like to ask for help to review the following change/update:

Issue: https://bugs.openjdk.java.net/browse/JDK-8229495
Webrev: http://cr.openjdk.java.net/~phedlin/tr8229495/


8229495: SIGILL in C2 generated OSR compilation

The approach to insert range-check guards (see, JDK-8193130, 
JDK-8216135, JDK-8240335) between the pre- and the main-loopis somewhat 
problematic. The immediate problem here is due to an inherent dependency 
between the additional (template) range-check guards introduced (during 
RCE) and the state of the loop, such as the level of loop-unrolling.To 
keep the range-check guards valid through the compilation, these 
arere-generated when/if the main-loop is unrolled further. Here, the 
error is introduced when a guard is generated with an illegal offset, 
that will erroneously cut the path to the main-loop (resulting in a 
'Halt'). The reason for range-checks to be present in the main-loop to 
begin with is due to a failing dominator search (this was also corrected 
in JDK-8231412, for JDK14).

Trying to solve this we could encode part of the loop state (e.g. the 
stride) into the templates when first introduced, in order to track the 
progress correctly. However, this still leaves us with a starting-point 
problem (here, we cannot tell what range-checks are actually valid 
starting-points). Also, the set of range-check guards generated from 
these new templates have a similar problem as the original loop-guard in 
that they will grow in complexity, possibly to a point where they may no 
longer be reduced at compile time.

The approach taken in this patch is to replace the range-check asserts 
with a more conservative check on the loop-variable staying inside the 
iteration space when passing between the pre- and the main-loop. The 
assert is introduced once (during RCE) and need not be updated to stay 
valid (it is supposed to be a proper tautology). The focus is 
correctness also in the degenerate case when the pre-loop depletes the 
iteration space. In particular, it will not determine the main-loop 
inaccessible in the cases when the main-loop would make to many 
iterations in its first trip (this could be supported by adjusting the 
upper bound of the conservative test to "ub - final-stride", after 
loop-optimisations are done).


Testing: hs-precheckin-comp, hs-tier1-6,8

Also including JDK11,12 and 14, however, the reproducing testcase has 
been ran (along with regressiontesting) using a set of tweaks(debug 
flags) for JDK14,15 to "open the path" to the kernel of the poodle (Des 
Pudels Kern). These tweaks are not included in the patch. (The 
reproducer working on JDK11,12 without tweaks.)


Best regards,
Patric