RFR: 8265129: Add intrinsic support for JVM.getClassId [v7]

Denghui Dong ddong at openjdk.java.net
Fri May 21 11:16:35 UTC 2021


On Fri, 21 May 2021 10:17:52 GMT, Markus Grönlund <mgronlun at openjdk.org> wrote:

>> Denghui Dong has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   update
>
> src/hotspot/share/jfr/jfr.cpp line 115:
> 
>> 113: }
>> 114: 
>> 115: #ifdef JFR_HAVE_INTRINSICS
> 
> Can make this unconditional from #ifdef JFR_HAVE_INTRINSICS, to simplify a bit.

fixed

> src/hotspot/share/jfr/jfr.hpp line 31:
> 
>> 29: #include "memory/allocation.hpp"
>> 30: 
>> 31: #include "jfr/support/jfrIntrinsics.hpp"
> 
> Don't think we need to include jfrIntrinsics.hpp (not even in .cpp) as we can declare this entry point unconditionally.

removed, thanks.

> src/hotspot/share/jfr/jfr.hpp line 33:
> 
>> 31: #include "jfr/support/jfrIntrinsics.hpp"
>> 32: #ifdef JFR_HAVE_INTRINSICS
>> 33: #include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp"
> 
> Please create a Jfr::epoch_address() in the .hpp and move includes to the .cpp - thanks.

fixed.

> src/hotspot/share/jfr/jfr.hpp line 63:
> 
>> 61:   static void include_thread(Thread* thread);
>> 62: 
>> 63: #ifdef JFR_HAVE_INTRINSICS
> 
> can be declared unconditionally to #ifdef JFR_HAVE_INTRINSICS (call site will have the conditional).

fixed.

> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdLoadBarrier.inline.hpp line 71:
> 
>> 69:     SET_USED_THIS_EPOCH(klass);
>> 70:     enqueue(klass);
>> 71:     JfrTraceIdEpoch::set_changed_tag_state();
> 
> JfrTraceIdEpoch::set_changed_tag_state(); here now obviates the need to program signal into the intrinsic so that can be removed. And signal_address() need not be exposed.

`load_barrier` is used by the path for normal class and `JfrTraceIdLoadBarrier::load(const Klass* klass)`
the path for the primitive class still needs to update the signal.

> src/hotspot/share/opto/library_call.cpp line 32:
> 
>> 30: #include "compiler/compileLog.hpp"
>> 31: #include "gc/shared/barrierSet.hpp"
>> 32: #include "jfr/jfr.hpp"
> 
> this needs #if INCLUDE_JFR guard

fixed

> src/hotspot/share/opto/library_call.cpp line 2820:
> 
>> 2818:     } __ end_if();
>> 2819: 
>> 2820:     Node* signaled_flag_address = makecon(TypeRawPtr::make(JfrTraceIdEpoch::signal_address()));
> 
> The signal stuff can now be removed. Please also remove JfrTraceIdEpoch::signal_address() exposure.

the path for the primitive class needs it, and I add Jfr::signal_address() to wrapper it

-------------

PR: https://git.openjdk.java.net/jdk/pull/3470


More information about the hotspot-compiler-dev mailing list