[10] RFR(M): 8176506: C2: loop unswitching and unsafe accesses cause crash

[Volker Simonis]

Hi Roland, Vladimir,

I'm currently implementing the ppc64/s390x pieces for this change.
Can you please give me a day or two before submitting it?

Thanks,
Volker


On Tue, May 2, 2017 at 7:20 PM, Vladimir Kozlov
<vladimir.kozlov at oracle.com> wrote:
> Looks good to me. I will submit RBT testing, lets see how it goes.
>
> Thanks,
> Vladimir
>
>
> On 4/28/17 1:46 AM, Roland Westrelin wrote:
>>
>>
>>> In addition to Andrew’s test i have a simple JMH microbenchmark i can
>>> send you, which i used to measure performance and identify issues with
>>> code generation, misalignment, and unrolled loops for byte buffer
>>> access (off/on heap) and VarHandle views. specjvm2008 is likely too
>>> coarse grained.
>>
>>
>> I found some regressions with Paul's tests but I can recover lost
>> performance using profiling data and some tweaks to the compiler. I'll
>> take care of all that as part of a follow up change.
>>
>> Here is a new webrev:
>>
>> http://cr.openjdk.java.net/~roland/8176506/webrev.03/
>>
>> I found I introduced a bug in LibraryCallKit::make_unsafe_address(): in
>> case of a raw memory access classify_unsafe_addr() changes base but we
>> don't want that change to propagate to
>> LibraryCallKit::inline_unsafe_access() otherwise it won't see a null
>> base for a raw memory access.
>>
>> If C2 compiles a method that makes incorrect use of Unsafe (by trying to
>> load a field from a null object for instance), the C2 HaltNode can be
>> executed. Its current implementation on several platforms result in a
>> call to os::breakpoint() which is an empty method. So execution would
>> happily return to the compiled method and continue and fail in some
>> mysterious way. That's why in my previous webrevs, I had implemented the
>> HaltNode on x86 and aarch64 as a call to a method that call
>> ShouldNotReachHere(). This needs to be taken care of on all
>> platforms. The webrev now includes an implementation on arm64 (that I
>> tested), arm32 and sparc (that I can't build or test). I would need
>> someone to verify them. I suppose PPC would need something similar as
>> well but I have limited PPC skills so I would need code for that
>> too. The easy way to test the halt implementation is to modify the
>> TestMaybeNullUnsafeAccess test from the patch and add a:
>>
>> test2(null);
>>
>> call after the loop of the main method. This should result in the Halt
>> node being executed.
>>
>> Roland.
>>
>