RFR: 8224974: Implement JEP 352

[Vladimir Kozlov]

Hotspot changes looks good.

CheckGraalIntrinsics failed because of typo - you should use 'J' instead of 'L':

+  "jdk/internal/misc/Unsafe.writeback0(L)V",

One nitpick in vmSymbols.hpp spacing is off in next line:

+  do_intrinsic(_writebackPostSync0,        jdk_internal_misc_Unsafe,     writebackPostSync0_name, void_method_signature 
, F_RN)   \

Thanks,
Vladimir

On 6/6/19 1:53 AM, Andrew Dinn wrote:
> Hi Brian,
> 
> On 06/06/2019 01:06, Brian Burkhalter wrote:
>> With version 6 [1] of the patch applied, tests throw this exception:
>>
>> Mapped java.lang.InternalError: java.lang.NullPointerException
>> at
>> java.base/jdk.internal.misc.ExtendedMapMode.newMapMode(ExtendedMapMode.java:58)
>> at
>> java.base/jdk.internal.misc.ExtendedMapMode.<clinit>(ExtendedMapMode.java:40)
>> at java.base/sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:1007)
>>
>> Looks like a problem in the initialization order of the static final
>> constants: MAP_MODE_CONSTRUCTOR needs to be initialized first so I think
>> the *_SYNC constants need to be after the static initializer [2].
> 
> Doh! Yes, you are right.
> 
> Sorry for adding confusion. webrev.06 was not meant to be intended for
> public consumption. I uploaded it last night when I set the rebuild off
> but I didn't intend to advertise it until I had retested and got your
> confirmation on the suggestions I made. Anyway, thanks all the same for
> reviewing it, trying to run it and also finding/fixing this issue.
> 
> I have updated webrev.06 in place and PmemTest now runs ok. I still need
> to have Jonathan Halliday test the patch with our Transaction and Data
> Grid software before it can be properly signed off. Another submit run
> will also be needed. However, PmemTest is a good enough sanity check to
> validate the changes you requested.
> 
> So, here is webrev.06 as the latest official patch:
> 
>    http://cr.openjdk.java.net/~adinn/8224974/webrev.06/
> 
> It should address
> 
>    changes requested on this list by Alan and Vladimir
> 
>    all the changes you requested in your review
>    (modulo those you agreed to defer on)
> 
>    the change to correct this static init order bug
> 
>    a change to graal test CheckGraalIntrinsics to declare
>    the new jdk13 intrinsics in alphanum sort order
>    (this was requested offline by Alan -- without it the test
>    refuses to run on jdk13)
> 
> The last of those changes still leaves one issue unanswered (although it
> may not actually be an issue). Vladimir is probably best able to comment.
> 
> When I run CheckGraalIntrinsics (which is actually run indirectly by
> compiler/graalunit/HotspotTest) with my patched tree it fails, claiming
> that Graal does not know about Unsafe.writeback0:
> 
>    java.lang.AssertionError: missing Graal intrinsics for:
>        jdk/internal/misc/Unsafe.writeback0(J)V
> 
> I am assuming this is what is expected to happen? Or perhaps the test is
> supposed to be added to the problem or exclude lists?
> 
>> Sorry for suggesting the change that was the proximate cause of this. :-(
> No problem :-) Thanks for reviewing and checking!
> 
> regards,
> 
> 
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>