RFR: 8203188: Add JEP-181 support to the Zero interpreter
David Holmes
david.holmes at oracle.com
Tue Jun 5 09:46:36 UTC 2018
Looks good.
I'll push this with the nestmate changes later in the week.
Thanks,
David
On 5/06/2018 7:40 PM, Severin Gehwolf wrote:
> Hi David,
>
> Thanks for the review!
>
> On Tue, 2018-06-05 at 14:44 +1000, David Holmes wrote:
>> Hi Severin,
>>
>> On 5/06/2018 1:26 AM, Severin Gehwolf wrote:
>>> Hi,
>>>
>>> Could I please get a review of this change adding support for JEP-181 -
>>> a.k.a Nestmates - to Zero. This patch depends on David Holmes'
>>> Nestmates implementation via JDK-8010319. Thanks to David Holmes and
>>> Chris Phillips for their initial reviews prior to this RFR.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8203188
>>> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203188/webrev.02/
>>
>> src/hotspot/cpu/zero/methodHandles_zero.cpp
>>
>> The change here seems to be an existing bug unrelated to nestmate
>> changes.
>
> Agreed.
>
>> IT also begs the question as to what happens in the same
>> circumstance with a removed static or "special" method? (I thought I had
>> a test for that in the nestmates changes ... will need to double-check
>> and add it if missing!).
>
> It might bomb in the same way (NULL dereference). I'm currently looking
> at some other potential issues in this area...
>
>> src/hotspot/share/interpreter/bytecodeInterpreter.cpp
>>
>> Interpreter changes seem fine - mirroring what is done elsewhere. You
>> can delete these incorrect comments:
>>
>> 2576 // This code isn't produced by javac, but could be produced by
>> 2577 // another compliant java compiler.
>>
>> That code path is taken in more circumstances than the author of that
>> comment realized. :)
>
> Done.
>
>>> Testing:
>>>
>>> Zero on Linux-x86_64 with the following test set:
>>>
>>> test/jdk/java/lang/invoke/AccessControlTest.java
>>> test/jdk/java/lang/invoke/FinalVirtualCallFromInterface.java
>>> test/jdk/java/lang/invoke/PrivateInterfaceCall.java
>>> test/jdk/java/lang/invoke/SpecialInterfaceCall.java
>>> test/jdk/java/lang/reflect/Nestmates
>>> test/hotspot/jtreg/runtime/SelectionResolution/InvokeInterfaceICCE.java
>>> test/hotspot/jtreg/runtime/SelectionResolution/InvokeInterfaceSuccessTest.java
>>> test/hotspot/jtreg/runtime/Nestmates
>>>
>>> I cannot run this through the submit repo since the main Nestmates
>>> patch hasn't yet landed in JDK 11. Currently testing a Zero bootcycle-
>>> images build on x86_64. Thoughts?
>
> FWIW, bootcycle-images build passed on linux x86_64 Zero.
>
>> I can bundle this in with the nestmate changes when I push them later
>> this week. Just send me a pointer to the finalized changeset once its
>> finalized. I'll run it all through a final step of testing equivalent
>> (actually more than) the submit repo.
>
> OK, thanks!
>
> Latest webrev:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203188/webrev.03/
>
> Thanks,
> Severin
>
>> Thanks,
>> David
>>
>>> Thanks,
>>> Severin
>>>
More information about the hotspot-dev
mailing list