RFR: 8156034: [JVMCI] Notify the jvmci compiler on completion of a bootstrap

Christian Thalinger christian.thalinger at oracle.com
Wed May 11 19:21:03 UTC 2016


> On May 10, 2016, at 9:39 PM, Josef Eisl <josef.eisl at jku.at> wrote:
> 
> Thanks again, Chris and Doug!
> 
> Next iteration of the webrev available here:
> http://cr.openjdk.java.net/~jeisl/8156034/webrev.02/

That looks good.

> 
> Thanks,
> Josef
> 
> On 10/05/16 23:35, Doug Simon wrote:
>> The _bootstrap_compilation_request_seen variable should be renamed to
>> _bootstrap_compilation_request_handled.
>> 
>> 
> 
> On 10/05/16 23:33, Christian Thalinger wrote:
>> 
>>> On May 10, 2016, at 11:20 AM, Josef Eisl <josef.eisl at jku.at
>>> <mailto:josef.eisl at jku.at>> wrote:
>>> 
>>> Thank you for the review!
>>> 
>>> update webrev: http://cr.openjdk.java.net/~jeisl/8156034/webrev.01
>>> <http://cr.openjdk.java.net/%7Ejeisl/8156034/webrev.01>
>> 
>> I think your change works but the proper way is to pass THREAD
>> to JVMCICompiler::bootstrap() and use CHECK here:
>> 
>> + JVMCIRuntime::bootstrap_finished(CHECK);
> 
> That makes sense. I'm still figuring out where to use which macro but
> I'm getting closer :).
> 
>>> 
>>> Thanks,
>>> Josef
>>> 
>>> On 05/10/2016 07:30 PM, Christian Thalinger wrote:
>>>> 
>>>>> On May 10, 2016, at 6:12 AM, Josef Eisl <josef.eisl at jku.at
>>>>> <mailto:josef.eisl at jku.at>
>>>>> <mailto:josef.eisl at jku.at>> wrote:
>>>>> 
>>>>> Hi!
>>>>> 
>>>>> May I get a review for this JVMCI change.
>>>>> 
>>>>> webrev: http://cr.openjdk.java.net/~jeisl/8156034/webrev/
>>>>> <http://cr.openjdk.java.net/%7Ejeisl/8156034/webrev/>
>>>> 
>>>> 
>>>>   src/share/vm/jvmci/jvmciCompiler.cpp
>>>> 
>>>> + _bootstrap_compilation_request_seen = false;
>>>> 
>>>> Remove the extra space after =
>>>> 
>>>> 
>>>>   src/share/vm/jvmci/jvmciCompiler.hpp
>>>> 
>>>> + * True if we have seen the a bootstrap compilation request.
>>>> 
>>>> Typo “the a”.
>>>> 
>>>> 
>>>>   src/share/vm/jvmci/jvmciRuntime.cpp
>>>> 
>>>> +void JVMCIRuntime::bootstrapFinished() {
>>>> 
>>>> Rename the method to bootstrap_finished.  Also, this method should use
>>>> TRAPS and propagate the exception.  In JNI_CreateJavaVM_inner use
>>>> something like this to report exceptions:
>>>> 
>>>> if (HAS_PENDING_EXCEPTION) {
>>>> HandleMark hm;
>>>>        vm_exit_during_initialization(Handle(THREAD, PENDING_EXCEPTION));
>>>>      }
>>>> 
>>>> + if (_HotSpotJVMCIRuntime_instance != NULL) {
>>>> 
>>>> Can this ever be not true at this point?
>>> 
>>> No, I guess you are right. In jdk9 this condition should always hold.
>>> 
>>>> 
>>>> 
>>>> *test/compiler/jvmci/events/JvmciNotifyBootstrapFinishedEventTest.java*
>>>> 
>>>>  27  * @requires (os.simpleArch == "x64" | os.simpleArch ==
>>>> "sparcv9" | os.simpleArch == "aarch64")
>>>>  28  * @requires (os.simpleArch == "x64" | os.simpleArch ==
>>>> "sparcv9" | os.simpleArch == "aarch64")
>>>> 
>>>> Remove duplicate line.
>>>> 
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8156034
>>>>> 
>>>>> It passes all jvmci tests.
>>>>> 
>>>>> Note that this changes depends on
>>>>> https://bugs.openjdk.java.net/browse/JDK-8155023
>>>>> 
>>>>> Thanks in advance,
>>>>> Josef
>> 
> 



More information about the hotspot-compiler-dev mailing list