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

[Roland Westrelin]

I took that bug over.
Thanks to Patric for helping me understand the root cause of the bug.

http://cr.openjdk.java.net/~roland/8229495/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8229495

> 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).

For a range check:

scale * i + offset <u length

the current code creates a "skeleton" predicate which is essentially an
assert:

assert scale * opaque(init) + offset <u length

As unrolling proceeds, the assert is duplicated and updated:

assert scale * opaque(init) + offset <u length
assert scale * init + offset <u length
assert scale * (init + stride - init_stride) + offset <u length

init_stride is the stride before any unrolling.

The goal is to cover the entire first iteration of the loop where
constant folding can occur and cause CastIIs to go dead in loop bodies
that are unreachable in practice but not optimized out.

This is broken if some rounds of unrolling happens before range check
elimination. If a loop with a range check:

scale * i + offset <u length

is unrolled once then we have 2 range checks:

scale * i + offset <u length
scale * (i+1) + offset <u length

Now range check elimination causes 2 predicates to be added:

assert scale * opaque(init) + offset <u length
assert scale * opaque(init) + (scale + offset) <u length

If the loop is unrolled one more time this is turned into:

assert scale * opaque(init) + offset <u length
assert scale * init + offset <u length
assert scale * (init + stride - init_stride) + offset <u length
assert scale * opaque(init) + (scale + offset) <u length
assert scale * init + (scale + offset) <u length
assert scale * (init + stride - init_stride) + (scale + offset) <u length

stride = 4, init_stride = 1 so the last assert is:

assert scale * (init + 4) + offset <u length

which for a loop unrolled 4 times is outside the first iteration.

The problem is that init_stride should be 2 here, the stride at which
range check elimination triggered. Given it can trigger at different
strides for different range checks, init_stride needs to be captured in
the skeleton. So I propose changing this scheme to:

For a range check:

scale * i + offset <u length

create 2 "skeleton" predicates:

assert scale * opaque(init) + offset <u length
assert scale * (opaque(init) + opaque(init_stride) - init_stride) + offset <u length

As unrolling proceeds, the asserts are duplicated and updated:

assert scale * opaque(init) + offset <u length
assert scale * init + offset <u length
assert scale * (opaque(init) + opaque(init_stride) - init_stride) + offset <u length
assert scale * (init + stride - init_stride) + offset <u length

I implemented this by using 2 subclasses to Opaque1 to denotate init and
stride and facilitate pattern matching. I had to extend _class_id to
juint to make Opaque1 a new class. I also had to tweak the budget
estimate code. Finally, the "asserts" above used to be removed at the
end of compilation. I now leave them in debug builds so we can catch
similar bugs earlier.

The bug doesn't reproduce with the included test case anymore but after
I backed out a couple unrelated changes I could use the test case to
verify the bug is indeed fixed.

And FWIW, I agree this has become way too complicated but I don't see a
simpler solution to the family of bugs this fixes.

Roland.