RFR 8242263: Diagnose synchronization on primitive wrappers

David Holmes david.holmes at oracle.com
Tue Aug 11 00:26:06 UTC 2020


One follow up ... I'll send separate mail for v5.

On 11/08/2020 6:19 am, Patricio Chilano wrote:
> Hi David,
> 
> On 8/10/20 1:12 AM, David Holmes wrote:
>> Hi Patricio,
>>
>> First, I confirmed with Valhalla folk that this change is fine to go 
>> ahead as a stand-alone change independent of the proposed JEP in this 
>> area.
> Great, thanks for that!  : )
> 
>> I've checked the incremental changes for v2, v3 and v4 and everything 
>> seems good to me.
>>
>> I only have a couple of nits with the tests:
>>
>>   54     private static void initTestObjects() {
>>   55         testObjects.add(Character.valueOf('H'));
>>   56         testObjects.add(Boolean.valueOf(true));
>>   57         Byte byteT = 0x40;
>>   58         testObjects.add(byteT);
>>   59         Short shortT = 0x4000;
>>   60         testObjects.add(shortT);
>>   61         testObjects.add(Integer.valueOf(0x40000000));
>>   62         testObjects.add(Long.valueOf(0x4000000000000000L));
>>   63         testObjects.add(Float.valueOf(1.20f));
>>   64         testObjects.add(Double.valueOf(1.2345));
>>   65     }
>>
>> Why are Byte and Short treated differently?
> If I do the same as with the other types, the argument passed to 
> Byte.valueOf() and Short.valueOf() gets treated as an int and I get a 
> compilation error:
> 
> : error: no suitable method found for valueOf(int)
>    testObjects.add(Byte.valueOf(0x40));

Right - you'd need to cast the constant to byte/short:

testObjects.add(Byte.valueOf((byte)0x40));

Cheers,
David
                                    ^
>    method Byte.valueOf(byte) is not applicable
>    (argument mismatch; possible lossy conversion from int to byte)
>    method Byte.valueOf(String) is not applicable
>    (argument mismatch; int cannot be converted to String)
> 
>>  74 fatalTests[index] = Stream.of(commonFatalTestsFlags, 
>> specificFlags[i], new String[] 
>> {"SyncOnPrimitiveWrapperTest$FatalTest", Integer.toString(j)})
>>   75 .flatMap(Stream::of).toArray(String[]::new);
>>
>> The preferred Java style [1] for stream operations is:
>>
>> fatalTests[index] = Stream.of(commonFatalTestsFlags, specificFlags[i], 
>> new String[] {"SyncOnPrimitiveWrapperTest$FatalTest", 
>> Integer.toString(j)})
>>                           .flatMap(Stream::of)
>>                           .toArray(String[]::new);
> Fixed.
> 
> Here is v5:
> Inc: http://cr.openjdk.java.net/~pchilanomate/8242263/v5/inc/webrev/
> Full: http://cr.openjdk.java.net/~pchilanomate/8242263/v5/webrev/
> 
> Thanks,
> Patricio
>> Thanks,
>> David
>> -----
>>
>> [1] As demonstrated here (but widely used in JDK code): 
>> http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html#toc-wrapping-lines 
>>
>>
>> On 7/08/2020 4:48 am, Patricio Chilano wrote:
>>> Hi Dan,
>>>
>>> On 8/5/20 6:47 PM, Daniel D. Daugherty wrote:
>>>> I'm peeking ahead to the next webrev... :-)
>>>>
>>>> > http://cr.openjdk.java.net/~pchilanomate/8242263/v3/webrev/
>>> : )
>>>
>>>> General comments:
>>>>   - check files for copyright year updates.
>>> Updated accessFlags.hpp, c1_MacroAssembler_aarch64.cpp, 
>>> c1_MacroAssembler_arm.cpp and c1_MacroAssembler_x86.cpp.
>>>
>>>> src/hotspot/share/runtime/synchronizer.hpp
>>>>     No comments.
>>>>
>>>> src/hotspot/share/runtime/synchronizer.cpp
>>>>     L507:   const markWord mark = obj->mark();
>>>>     L508:
>>>>     L509:   if (DiagnoseSyncOnPrimitiveWrappers != 0 && 
>>>> obj->klass()->is_box()) {
>>>>     L510:     return false;
>>>>     L511:   }
>>>>         The new bailout on L509-511 can move above L507.
>>> Moved.
>>>
>>>> L573:     fatal("Synchronizing on object " INTPTR_FORMAT " of klass 
>>>> %s", p2i(obj()), obj->klass()->external_name());
>>>>         This external_name() call does not have a ResourceMark.
>>> Good catch! I had one in a previous version but then I changed the 
>>> conditionals and lost it for the fatal error case. The test worked 
>>> okay because for the main JavaThread there is a ResourceMark in 
>>> jni_invoke_static() (jni.cpp).
>>>
>>>> src/hotspot/share/logging/logTag.hpp
>>>>     No comments.
>>>>
>>>> src/hotspot/share/oops/klass.hpp
>>>>     No comments.
>>>>
>>>> src/hotspot/share/utilities/accessFlags.hpp
>>>>     No comments.
>>>>
>>>> src/hotspot/share/runtime/globals.hpp
>>>>     L814:              "0: off "
>>>>         Missing a ';' after "off".
>>> Fixed.
>>>
>>>> L816:              "2: log message to stdout.
>>>>         Perhaps add "(by default)" after "stdout" or
>>>>         don't say where log output is at all.
>>>>
>>>> src/hotspot/share/runtime/arguments.cpp
>>>>     L4197: LogConfiguration::configure_stdout(LogLevel::Info, true, 
>>>> LOG_TAGS(primitivewrappers));
>>>>         Hmmm... maybe ignore my comments about L816 in globals.hpp
>>>>         since it looks like logging output is configured to 'stdout'.
>>>>         I'm assuming that other log options to put output elsewhere
>>>>         are overridden by this code?
>>> Right. So the logging is done under UL with the tag 
>>> primitivewrappers. If that tag is specified in Xlog then this 
>>> conditional will be skipped because !log_is_enabled(Info, 
>>> primitivewrappers) will be false.
>>>
>>>> src/hotspot/share/classfile/systemDictionary.cpp
>>>>     L2188:     for (int i = T_BOOLEAN; i < T_LONG+1; i++) {
>>>>         nit - s/T_LONG+1/T_LONG + 1/
>>> Fixed.
>>>
>>>> L2191: _box_klasses[i]->set_prototype_header(markWord::prototype());
>>>>         I assume we're keeping the prototype_header field when 
>>>> Biased Locking
>>>>         goes away? The reason I ask:
>>>>
>>>>         static markWord prototype() {
>>>>           return markWord( no_hash_in_place | no_lock_in_place );
>>>>         }
>>>>
>>>>         is because without Biased Locking, do we really need the 
>>>> prototype
>>>>         anymore? The initial markWord won't need possible variants...
>>> Yes, I think it can go away unless somebody finds another use for it. 
>>> Maybe Valhalla.
>>>
>>>> src/hotspot/share/jfr/metadata/metadata.xml
>>>>     L69:   <Event name="SyncOnPrimitiveWrapper" category="Java 
>>>> Application"
>>>>         Is the category "Java Application" because it's the application
>>>>         code that did something "wrong" here? Where "application" is 
>>>> loosely
>>>>         defined to include the possibility of auto generated code, 
>>>> library
>>>>         code and the like? Or perhaps "application" because 
>>>> something "above"
>>>>         the "Java Virtual Machine, Runtime" did the "wrong" thing here?
>>> I don't know what the right category should be really. I saw the 
>>> events JavaMonitorEnter, JavaMonitorWait and JavaMonitorInflate and 
>>> thought this event should fall in the same category they do.
>>>
>>>> src/jdk.jfr/share/conf/jfr/default.jfc
>>>>     No comments.
>>>>
>>>> src/jdk.jfr/share/conf/jfr/profile.jfc
>>>>     No comments.
>>>>
>>>> test/lib/jdk/test/lib/jfr/EventNames.java
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/aarch64/aarch64.ad
>>>>     L3517:       __ tbnz(tmp, exact_log2(JVM_ACC_IS_BOX_CLASS), cont);
>>>> <snip>
>>>>     L3578:     __ bind(cont);
>>>>     L3579:     // flag == EQ indicates success
>>>>     L3580:     // flag == NE indicates failure
>>>>         If tbnz() branches to "cont" when we have a box class, what's
>>>>         the flag value set to (EQ or NE)? And what set that flag value?
>>>>         The reason I ask is I don't think tbnz() sets condition 
>>>> codes...
>>> Right, it doesn't set condition codes, so I kept the version I had 
>>> (more on that below).
>>>
>>>> src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
>>>>     int MacroAssembler::biased_locking_enter(Register lock_reg,
>>>>                                              Register obj_reg,
>>>>                                              Register swap_reg,
>>>>                                              Register tmp_reg,
>>>>                                              bool 
>>>> swap_reg_contains_mark,
>>>>                                              Label& done,
>>>>                                              Label* slow_case,
>>>> BiasedLockingCounters* counters) {
>>>>         I think you've changed the only callers of 
>>>> biased_locking_enter()
>>>>         that cared about the return value with this changeset so it can
>>>>         be changed to a void function.
>>> Ok, this is what I mentioned to David in a previous email. Done.
>>>
>>>> src/hotspot/cpu/arm/c1_MacroAssembler_arm.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/arm/c2_MacroAssembler_arm.cpp
>>>>     L96:      tbnz(Rscratch, exact_log2(JVM_ACC_IS_BOX_CLASS), done);
>>>> <snip>
>>>>     L131:   bind(done);
>>>>     L132:
>>>>     L133:   // At this point flags are set as follows:
>>>>     L134:   //  EQ -> Success
>>>>     L135:   //  NE -> Failure, branch to slow path
>>>>         If tbnz() branches to "done" when we have a box class, what's
>>>>         the flag value set to (EQ or NE)? And what set that flag value?
>>>>         The reason I ask is I don't think tbnz() sets condition 
>>>> codes...
>>> Right. Same as above, I kept the version I had.
>>>
>>>> src/hotspot/cpu/arm/interp_masm_arm.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/arm/macroAssembler_arm.cpp
>>>>     int MacroAssembler::biased_locking_enter(Register obj_reg, 
>>>> Register swap_reg, Register tmp_reg,
>>>>                                              bool 
>>>> swap_reg_contains_mark,
>>>>                                              Register tmp2,
>>>>                                              Label& done, Label& 
>>>> slow_case,
>>>> BiasedLockingCounters* counters) {
>>>>         I think you've changed the only callers of 
>>>> biased_locking_enter()
>>>>         that cared about the return value with this changeset so it can
>>>>         be changed to a void function.
>>> Done.
>>>
>>>> src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp
>>>> src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp
>>>> src/hotspot/cpu/ppc/macroAssembler_ppc.cpp
>>>>     No comments on the PPC code.
>>>>
>>>> src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp
>>>> src/hotspot/cpu/s390/interp_masm_s390.cpp
>>>> src/hotspot/cpu/s390/macroAssembler_s390.cpp
>>>>     No comments on the S390 code.
>>>>
>>>> src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp
>>>>     L58:     load_klass(hdr, obj, rklass_decode_tmp);
>>>>         What will this do with a 'noreg' value? (I need a refresher.)
>>> When not in 64 bit mode that register just won't be used.
>>>
>>>> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/x86/interp_masm_x86.cpp
>>>>     No comments.
>>>>
>>>> src/hotspot/cpu/x86/macroAssembler_x86.cpp
>>>>     int MacroAssembler::biased_locking_enter(Register lock_reg,
>>>>                                              Register obj_reg,
>>>>                                              Register swap_reg,
>>>>                                              Register tmp_reg,
>>>>                                              Register tmp_reg2,
>>>>                                              bool 
>>>> swap_reg_contains_mark,
>>>>                                              Label& done,
>>>>                                              Label* slow_case,
>>>> BiasedLockingCounters* counters) {
>>>>         I think you've changed the only caller of 
>>>> biased_locking_enter()
>>>>         that cared about the return value with this changeset so it can
>>>>         be changed to a void function.
>>> Done.
>>>
>>>> test/hotspot/jtreg/runtime/Monitor/SyncOnPrimitiveWrapperTest.java
>>>>     L30:  * @test SyncOnPrimitiveWrapperTest
>>>>         No parameter to @test directive.
>>> Removed.
>>>
>>>> L136:         private static long sharedCounter = 0L;
>>>>         Since you don't do anything with sharedCounter other than 
>>>> increment it,
>>>>         can the compilers optimize it away? If it can be optimized 
>>>> away, does
>>>>         that mean that:
>>>>
>>>>             L142:                 synchronized (obj) {
>>>>
>>>>         can also be optimized away?
>>>>
>>>>         I don't think that:
>>>>
>>>>             L161:                 synchronized (sharedLock1) {
>>>>
>>>>         can be optimized away because it is shared by multiple threads.
>>>>
>>>> test/jdk/jdk/jfr/event/runtime/TestSyncOnPrimitiveWrapperEvent.java
>>>>     Similar questions about 'counter' being optimized away.
>>>>     Similar question about "synchronized (obj) {" being optimized away.
>>> I'm not sure if the compiler will optimize it away. Seems unlikely 
>>> given the counter we are incrementing is not just local to some thread.
>>>
>>>
>>> Ok, below is v3 which has the following changes:
>>> - Use a 32 bit load for the _access_flags field, instead of 64
>>> - Martin's implementation for s390 and fix for PPC
>>> - Faster LogTest version
>>> - Above changes based on Dan review
>>>
>>> I'm retesting in mach5 tiers1-6 (which tests x64 and aarch64 only) 
>>> again with -XX:DiagnoseSyncOnPrimitiveWrappers=2. I checked it builds 
>>> in arm32, ppc and s390.
>>>
>>> Full: http://cr.openjdk.java.net/~pchilanomate/8242263/v3/webrev
>>> Inc: http://cr.openjdk.java.net/~pchilanomate/8242263/v3/inc/webrev
>>>
>>>
>>> @Martin:
>>> Please check if the test doesn't timeout for you now with the changes 
>>> I made to LogTest.
>>> Also, I tried to use tbnz in aarch64 and arm32 instead of tst + br 
>>> (except for c2 since we actually need to set the condition flags), 
>>> but for c1 I was getting an assertion in the compiler thread:
>>>
>>> guarantee(chk == -1 || chk == 0) failed: Field too big for insn
>>>
>>> This is the stack when that happens:
>>>
>>> MacroAssembler::pd_patch_instruction_size(unsigned char*, unsigned 
>>> char*)+0x398
>>> AbstractAssembler::bind(Label&)+0x118
>>> MonitorEnterStub::emit_code(LIR_Assembler*)+0x28
>>> LIR_Assembler::emit_slow_case_stubs()+0x54
>>> Compilation::emit_code_body()+0x17c
>>>
>>> The change was simply:
>>>
>>> diff --git a/src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp 
>>> b/src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp
>>> --- a/src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp
>>> +++ b/src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp
>>> @@ -78,7 +78,6 @@
>>>     if (DiagnoseSyncOnPrimitiveWrappers != 0) {
>>>       load_klass(hdr, obj);
>>> -    ldr(hdr, Address(hdr, Klass::access_flags_offset()));
>>> -    tst(hdr, JVM_ACC_IS_BOX_CLASS);
>>> -    br(Assembler::NE, slow_case);
>>> +    ldrw(hdr, Address(hdr, Klass::access_flags_offset()));
>>> +    tbnz(hdr, exact_log2(JVM_ACC_IS_BOX_CLASS), slow_case);
>>>     }
>>>
>>> So the issue must be related to where we want to jump.
>>>
>>> Thanks,
>>> Patricio
>>>> Dan
>>>>
>>>>
>>>> On 8/5/20 9:01 AM, Patricio Chilano wrote:
>>>>> Hi Martin,
>>>>>
>>>>> On 8/5/20 4:47 AM, Doerr, Martin wrote:
>>>>>> Hi Patricio,
>>>>>>
>>>>>> using 8 Byte load instructions for jint fields is a terrible 
>>>>>> coding style!
>>>>>> Someone else may see it and use an 8 Byte store. Will result in 
>>>>>> great fun for debugging!
>>>>>>
>>>>>> And on Big Endian you will simply access the wrong bits.
>>>>> Ah, of course! Those 32 bits will be in the wrong place when doing 
>>>>> the test.
>>>>>
>>>>>> Note that Big Endian Platforms are AIX, old linux ppc, s390, 
>>>>>> SPARC. I don't think you have tested on them.
>>>>>>
>>>>>>> We could remove the nested synchronized statements in the run() 
>>>>>>> method
>>>>>>> so that we don't generate that much logging. We could also lower
>>>>>>> LOOP_COUNT. I think the issue is also because we are running LogTest
>>>>>>> with multiple flag combinations. Not sure what we should touch 
>>>>>>> first.
>>>>>>> Maybe the synchronized statements, have only one or two and test 
>>>>>>> that
>>>>>>> first?
>>>>>> Sounds like helpful ideas. Please go ahead and strip things down.
>>>>> Great, I will send v3 later with those changes.
>>>>>
>>>>> Thanks Martin!
>>>>>
>>>>> Patricio
>>>>>> Thanks for taking care of it.
>>>>>>
>>>>>> Best regards,
>>>>>> Martin
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Patricio Chilano <patricio.chilano.mateo at oracle.com>
>>>>>>> Sent: Dienstag, 4. August 2020 20:26
>>>>>>> To: Doerr, Martin <martin.doerr at sap.com>; David Holmes
>>>>>>> <david.holmes at oracle.com>; hotspot-runtime-dev at openjdk.java.net;
>>>>>>> hotspot-jfr-dev at openjdk.java.net
>>>>>>> Subject: Re: RFR 8242263: Diagnose synchronization on primitive 
>>>>>>> wrappers
>>>>>>>
>>>>>>> Hi Martin,
>>>>>>>
>>>>>>> On 8/4/20 1:35 PM, Doerr, Martin wrote:
>>>>>>>> Hi Patricio,
>>>>>>>>
>>>>>>>>> Ok, I'll change it to movl + testl and test it out before 
>>>>>>>>> sending v3.
>>>>>>>> Thanks. I forgot to mention arm + aarch64.
>>>>>>>> Aarch64 uses ldrw + tbnz.
>>>>>>>> Arm 32 bit uses ldr_u32 + tbnz.
>>>>>>>>
>>>>>>>> I remember having seen the same mistake ��
>>>>>>>> And nobody noticed it on little Endian platforms.
>>>>>>> Ok, I can use a tbnz instead of test and then a branch on arm32 
>>>>>>> and aarch64.
>>>>>>> Shouldn't a normal ldr in arm32 work fine?
>>>>>>> Also in 64 bits (either x64 or aarch64) I don't see the issue of 
>>>>>>> using a
>>>>>>> 64 bit load, besides the fact that we only care about the first 
>>>>>>> 32 bits.
>>>>>>> Regardless of the endianness, aren't we masking out the upper 
>>>>>>> part when
>>>>>>> we do AND/TEST or if we test a bit in the 0-31 bit range? 
>>>>>>> Otherwise it
>>>>>>> seems it should have failed for one of those platforms in my tests.
>>>>>>>
>>>>>>>>> I see that some tests use @run driver/timeout=xxxx. Maybe you can
>>>>>>>>> specify that and see if that fixes it? Let me know if that 
>>>>>>>>> works and I
>>>>>>>>> can add it to the test.
>>>>>>>> That seems to have an effect. But now I'm not patient enough to 
>>>>>>>> wait for
>>>>>>> the test to finish.
>>>>>>>> Maybe the problem is that I'm using slow debug builds.
>>>>>>>> But is there a chance to make the test quicker without losing 
>>>>>>>> coverage
>>>>>>> significantly?
>>>>>>>> I think the test is too slow this way to run it on a regular 
>>>>>>>> basis. We'd need to
>>>>>>> spend dedicated machines for it.
>>>>>>> We could remove the nested synchronized statements in the run() 
>>>>>>> method
>>>>>>> so that we don't generate that much logging. We could also lower
>>>>>>> LOOP_COUNT. I think the issue is also because we are running LogTest
>>>>>>> with multiple flag combinations. Not sure what we should touch 
>>>>>>> first.
>>>>>>> Maybe the synchronized statements, have only one or two and test 
>>>>>>> that
>>>>>>> first?
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Patricio
>>>>>>>> Best regards,
>>>>>>>> Martin
>>>>>>>>
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Patricio Chilano <patricio.chilano.mateo at oracle.com>
>>>>>>>>> Sent: Dienstag, 4. August 2020 17:47
>>>>>>>>> To: Doerr, Martin <martin.doerr at sap.com>; David Holmes
>>>>>>>>> <david.holmes at oracle.com>; hotspot-runtime-dev at openjdk.java.net;
>>>>>>>>> hotspot-jfr-dev at openjdk.java.net
>>>>>>>>> Subject: Re: RFR 8242263: Diagnose synchronization on primitive 
>>>>>>>>> wrappers
>>>>>>>>>
>>>>>>>>> Hi Martin,
>>>>>>>>>
>>>>>>>>> Thanks for fixing PPC and taking care of s390!
>>>>>>>>>
>>>>>>>>> On 8/4/20 11:18 AM, Doerr, Martin wrote:
>>>>>>>>>> Hi Patricio,
>>>>>>>>>>
>>>>>>>>>> I suggest to use movl + testl for checking the access flag as 
>>>>>>>>>> for other
>>>>>>> access
>>>>>>>>> flags on x64.
>>>>>>>>> Ok, I'll change it to movl + testl and test it out before 
>>>>>>>>> sending v3.
>>>>>>>>>
>>>>>>>>>> New version for PPC64 and s390 see below.
>>>>>>>>>>
>>>>>>>>>> The test SyncOnPrimitiveWrapperTest produces hs_err files as
>>>>>>> expected.
>>>>>>>>> However, I'm getting timeout issues:
>>>>>>>>>> timed out (timeout set to 120000ms, elapsed time including 
>>>>>>>>>> timeout
>>>>>>>>> handling was 122372ms)
>>>>>>>>>> Can we provide more time?
>>>>>>>>> I see that some tests use @run driver/timeout=xxxx. Maybe you can
>>>>>>>>> specify that and see if that fixes it? Let me know if that 
>>>>>>>>> works and I
>>>>>>>>> can add it to the test.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Patricio
>>>>>>>>>> Best regards,
>>>>>>>>>> Martin
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> diff -r 77852e129401 
>>>>>>>>>> src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp
>>>>>>>>>> --- a/src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp Tue Aug 04
>>>>>>>>> 10:03:57 2020 +0200
>>>>>>>>>> +++ b/src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp Tue Aug 04
>>>>>>>>> 16:04:31 2020 +0200
>>>>>>>>>> @@ -107,8 +107,8 @@
>>>>>>>>>>
>>>>>>>>>>       if (DiagnoseSyncOnPrimitiveWrappers != 0) {
>>>>>>>>>>         load_klass(Rscratch, Roop);
>>>>>>>>>> -    ld(Rscratch, in_bytes(Klass::access_flags_offset()), 
>>>>>>>>>> Rscratch);
>>>>>>>>>> -    andi(Rscratch, Rscratch, JVM_ACC_IS_BOX_CLASS);
>>>>>>>>>> +    lwz(Rscratch, in_bytes(Klass::access_flags_offset()), 
>>>>>>>>>> Rscratch);
>>>>>>>>>> +    testbitdi(CCR0, R0, Rscratch, 
>>>>>>>>>> exact_log2(JVM_ACC_IS_BOX_CLASS));
>>>>>>>>>>         bne(CCR0, slow_case);
>>>>>>>>>>       }
>>>>>>>>>>
>>>>>>>>>> diff -r 77852e129401 src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp
>>>>>>>>>> --- a/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp Tue Aug 04
>>>>>>>>> 10:03:57 2020 +0200
>>>>>>>>>> +++ b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp Tue Aug 04
>>>>>>>>> 16:04:31 2020 +0200
>>>>>>>>>> @@ -912,8 +912,8 @@
>>>>>>>>>>
>>>>>>>>>>         if (DiagnoseSyncOnPrimitiveWrappers != 0) {
>>>>>>>>>>           load_klass(tmp, object);
>>>>>>>>>> -      ld(tmp, in_bytes(Klass::access_flags_offset()), tmp);
>>>>>>>>>> -      andi(tmp, tmp, JVM_ACC_IS_BOX_CLASS);
>>>>>>>>>> +      lwz(tmp, in_bytes(Klass::access_flags_offset()), tmp);
>>>>>>>>>> +      testbitdi(CCR0, R0, tmp, 
>>>>>>>>>> exact_log2(JVM_ACC_IS_BOX_CLASS));
>>>>>>>>>>           bne(CCR0, slow_case);
>>>>>>>>>>         }
>>>>>>>>>>
>>>>>>>>>> diff -r 77852e129401 src/hotspot/cpu/ppc/macroAssembler_ppc.cpp
>>>>>>>>>> --- a/src/hotspot/cpu/ppc/macroAssembler_ppc.cpp Tue Aug 04
>>>>>>>>> 10:03:57 2020 +0200
>>>>>>>>>> +++ b/src/hotspot/cpu/ppc/macroAssembler_ppc.cpp Tue Aug 04
>>>>>>>>> 16:04:31 2020 +0200
>>>>>>>>>> @@ -2838,8 +2838,8 @@
>>>>>>>>>>
>>>>>>>>>>       if (DiagnoseSyncOnPrimitiveWrappers != 0) {
>>>>>>>>>>         load_klass(temp, oop);
>>>>>>>>>> -    ld(temp, in_bytes(Klass::access_flags_offset()), temp);
>>>>>>>>>> -    andi(temp, temp, JVM_ACC_IS_BOX_CLASS);
>>>>>>>>>> +    lwz(temp, in_bytes(Klass::access_flags_offset()), temp);
>>>>>>>>>> +    testbitdi(CCR0, R0, temp, exact_log2(JVM_ACC_IS_BOX_CLASS));
>>>>>>>>>>         bne(CCR0, cont);
>>>>>>>>>>       }
>>>>>>>>>>
>>>>>>>>>> diff -r 77852e129401
>>>>>>> src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp
>>>>>>>>>> --- a/src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp Tue Aug 04
>>>>>>>>> 10:03:57 2020 +0200
>>>>>>>>>> +++ b/src/hotspot/cpu/s390/c1_MacroAssembler_s390.cpp Tue Aug 04
>>>>>>>>> 16:04:31 2020 +0200
>>>>>>>>>> @@ -91,6 +91,12 @@
>>>>>>>>>>       // Save object being locked into the BasicObjectLock...
>>>>>>>>>>       z_stg(obj, Address(disp_hdr,
>>>>>>> BasicObjectLock::obj_offset_in_bytes()));
>>>>>>>>>> +  if (DiagnoseSyncOnPrimitiveWrappers != 0) {
>>>>>>>>>> +    load_klass(Z_R1_scratch, obj);
>>>>>>>>>> +    testbit(Address(Z_R1_scratch, Klass::access_flags_offset()),
>>>>>>>>> exact_log2(JVM_ACC_IS_BOX_CLASS));
>>>>>>>>>> +    z_btrue(slow_case);
>>>>>>>>>> +  }
>>>>>>>>>> +
>>>>>>>>>>       if (UseBiasedLocking) {
>>>>>>>>>>         biased_locking_enter(obj, hdr, Z_R1_scratch, 
>>>>>>>>>> Z_R0_scratch, done,
>>>>>>>>> &slow_case);
>>>>>>>>>>       }
>>>>>>>>>> diff -r 77852e129401 src/hotspot/cpu/s390/interp_masm_s390.cpp
>>>>>>>>>> --- a/src/hotspot/cpu/s390/interp_masm_s390.cpp Tue Aug 04 
>>>>>>>>>> 10:03:57
>>>>>>>>> 2020 +0200
>>>>>>>>>> +++ b/src/hotspot/cpu/s390/interp_masm_s390.cpp Tue Aug 04
>>>>>>> 16:04:31
>>>>>>>>> 2020 +0200
>>>>>>>>>> @@ -1000,6 +1000,12 @@
>>>>>>>>>>       // Load markWord from object into displaced_header.
>>>>>>>>>>       z_lg(displaced_header, oopDesc::mark_offset_in_bytes(), 
>>>>>>>>>> object);
>>>>>>>>>>
>>>>>>>>>> +  if (DiagnoseSyncOnPrimitiveWrappers != 0) {
>>>>>>>>>> +    load_klass(Z_R1_scratch, object);
>>>>>>>>>> +    testbit(Address(Z_R1_scratch, Klass::access_flags_offset()),
>>>>>>>>> exact_log2(JVM_ACC_IS_BOX_CLASS));
>>>>>>>>>> +    z_btrue(slow_case);
>>>>>>>>>> +  }
>>>>>>>>>> +
>>>>>>>>>>       if (UseBiasedLocking) {
>>>>>>>>>>         biased_locking_enter(object, displaced_header, Z_R1, 
>>>>>>>>>> Z_R0, done,
>>>>>>>>> &slow_case);
>>>>>>>>>>       }
>>>>>>>>>> diff -r 77852e129401 src/hotspot/cpu/s390/macroAssembler_s390.cpp
>>>>>>>>>> --- a/src/hotspot/cpu/s390/macroAssembler_s390.cpp Tue Aug 04
>>>>>>>>> 10:03:57 2020 +0200
>>>>>>>>>> +++ b/src/hotspot/cpu/s390/macroAssembler_s390.cpp Tue Aug 04
>>>>>>>>> 16:04:31 2020 +0200
>>>>>>>>>> @@ -3358,6 +3358,12 @@
>>>>>>>>>>       // Load markWord from oop into mark.
>>>>>>>>>>       z_lg(displacedHeader, 0, oop);
>>>>>>>>>>
>>>>>>>>>> +  if (DiagnoseSyncOnPrimitiveWrappers != 0) {
>>>>>>>>>> +    load_klass(Z_R1_scratch, oop);
>>>>>>>>>> +    testbit(Address(Z_R1_scratch, Klass::access_flags_offset()),
>>>>>>>>> exact_log2(JVM_ACC_IS_BOX_CLASS));
>>>>>>>>>> +    z_btrue(done);
>>>>>>>>>> +  }
>>>>>>>>>> +
>>>>>>>>>>       if (try_bias) {
>>>>>>>>>>         biased_locking_enter(oop, displacedHeader, temp, Z_R0, 
>>>>>>>>>> done);
>>>>>>>>>>       }
>>>>>>>>>>
>>>>>
>>>>
>>>
> 


More information about the hotspot-runtime-dev mailing list