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

[Vladimir Kozlov]

Testing passed.

On 5/3/17 9:30 AM, Volker Simonis wrote:
> 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?

Okay. No problem.

Thanks,
Vladimir

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