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

Markus Grönlund mgronlun at openjdk.java.net
Fri May 21 10:28:35 UTC 2021


On Fri, 21 May 2021 08:18:55 GMT, Denghui Dong <ddong at openjdk.org> wrote:

>> 8265129: Add intrinsic support for JVM.getClassId
>
> Denghui Dong has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update

Thanks, Denghui - I saw a few more simplifications. Thank you for accommodating.

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.

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.

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.

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

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.

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

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.

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

Changes requested by mgronlun (Reviewer).

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


More information about the hotspot-compiler-dev mailing list