RFR (XL) 8031320: Use Intel RTM instructions for locks

Vladimir Kozlov vladimir.kozlov at oracle.com
Fri Mar 21 00:46:20 UTC 2014


Thank you, Dan

I fixed what you suggested and answered your questions. I will push 
these changes into our jdk9 hs-comp repo since other reviewers said okay.

Thanks,
Vladimir

On 3/20/14 3:49 PM, Daniel D. Daugherty wrote:
> On 3/19/14 11:26 PM, Vladimir Kozlov wrote:
>> Thank you, Dan
>>
>> I updated webrev.01 with your and last Igor's suggestions. See my
>> other mail with description of changes in it (mostly refactoring in
>> macroAssembler_x86.cpp).
>>
>> http://cr.openjdk.java.net/~kvn/8031320_9/webrev.01/
>>
>
> src/share/vm/runtime/rtmLocking.hpp
>      line 88: static uintx* rtm_calculation_flag_adr() { return
> &_calculation_flag; }
>          Typo: 'rtm_calculation_flag_adr' -> 'rtm_calculation_flag_addr'

Fixed

>
> src/cpu/x86/vm/assembler_x86.cpp
>      line 3040: void Assembler::xend() {
>          nit: Looks like these functions are in alpha order so xend()
>          should be before xgetbv(). Sorry I missed this in the previous
>          round.

Fixed.

> src/share/vm/opto/graphKit.cpp
>      line 3154: if (UseBiasedLocking &&
> PrintPreciseBiasedLockingStatistics) {
>          This looks like another more general fix that should be
>          backported.

Agree.

>
>      line 3160: flock->create_rtm_lock_counter(sync_jvms()); //
> sync_jvms used to get current bci
>          Why are the counters always created?

I moved checks when to create counters inside create_rtm_lock_counter() 
method because I did not want to add #if into graphkit.cpp. Counters 
will be created when:
(C->profile_rtm() || (PrintPreciseRTMLockingStatistics && C->use_rtm())

> src/share/vm/opto/macro.hpp
>      The placement of the new _has_locks field seems strange.
>      Other fields are above the functions.

You are right. I moved it after ProjNode *_resproj; field.

>
> src/share/vm/opto/runtime.hpp
>      line 90: assert(_next == NULL || next == NULL, "already set");
>          This looks like another more general fix that should be
>          backported.

I will collect all your notes what we should backport into 7u.

>
> src/share/vm/opto/runtime.cpp
>      line 1360   } else if (tag == NamedCounter::RTMLockingCounter) {
>      line 1361     c = new RTMLockingNamedCounter(strdup(st.as_string()));
>          Previous new code is in #if INCLUDE_RTM_OPT ... #endif
>          Is there some reason for the difference?

RTMLockingCounters* is defined in rtmLocking.hpp and its methods which 
are used in guarded code are defined only in rtmLocking.cpp on x86.

The enum NamedCounter::RTMLockingCounter and RTMLockingNamedCounter are 
defined on all platforms in runtime.hpp.

>
>      line 1370:  c->set_next(NULL);
>          This looks like another more general fix that should be
>          backported.

Yes, it is old bug which I fixed and needs to be backported.

>
> src/share/vm/opto/type.cpp
>      line 4383: return make(ptr, _metadata, offset);
>          This does not seem to be RTM related and I don't see that
>          file modified in the original webrev. Is this an unrelated
>          change that managed to sneak in?

It is related. I said about it in one of previous mails:

"In phase1.cpp I changed to correct TypeMetadataPtr for MDO pointer 
instead of original RawPtr. And I hit bug in TypeMetadataPtr::xmeet() 
and fixed it."

It is general bug but it affects only 9 and 8u with removed PermGen. It 
will be fixed with backport of these changes.

Thanks,
Vladimir

>
> Dan
>
>


More information about the hotspot-dev mailing list