Re: RFR: 8260899: ARM32: SyncOnValueBasedClassTest fails with assert(is_valid()) failed: invalid register

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8260899: ARM32: SyncOnValueBasedClassTest fails with assert(is_valid()) failed: invalid register

Boris Ulasevich-2
On Tue, 2 Feb 2021 09:24:20 GMT, Aleksey Shipilev <[hidden email]> wrote:

> $ CONF=linux-arm-server-fastdebug make run-test TEST=runtime/Monitor/SyncOnValueBasedClassTest.java
> ...
>
> #  Internal Error (/home/pi/jdk/src/hotspot/cpu/arm/register_arm.hpp:155), pid=3793, tid=3808
> #  assert(is_valid()) failed: invalid register
> #
> # JRE version: OpenJDK Runtime Environment (17.0) (fastdebug build 17-internal+0-adhoc.pi.jdk)
> # Java VM: OpenJDK Server VM (fastdebug 17-internal+0-adhoc.pi.jdk, compiled mode, emulated-client, g1 gc, linux-arm)
> # Problematic frame:
> # V  [libjvm.so+0xe1a6a8]  MacroAssembler::load_klass(RegisterImpl*, RegisterImpl*, AsmCondition)+0xa4
>
> Current CompileTask:
> C1:    318    2   !b        java.lang.Class::desiredAssertionStatus (54 bytes)
>
> Stack: [0x72580000,0x72600000],  sp=0x725fe170,  free space=504k
> Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
> V  [libjvm.so+0xe1a6a8]  MacroAssembler::load_klass(RegisterImpl*, RegisterImpl*, AsmCondition)+0xa4
> V  [libjvm.so+0x43b6b4]  C1_MacroAssembler::lock_object(RegisterImpl*, RegisterImpl*, RegisterImpl*, RegisterImpl*, Label&)+0xcf8
> V  [libjvm.so+0x3d731c]  LIR_Assembler::emit_lock(LIR_OpLock*)+0x160
>
>
> The problem is in this code:
>
>   if (DiagnoseSyncOnValueBasedClasses != 0) {
>     load_klass(tmp1, obj); <--- asserts
>     ldr_u32(tmp1, Address(tmp1, Klass::access_flags_offset()));
>     tst(tmp1, JVM_ACC_IS_VALUE_BASED_CLASS);
>     b(slow_case, ne);
>   }
>
> `tmp1` is `noreg` when `!BiasedLocking`, because `c1_LIRGenerator_arm.cpp` provides it only when `UseBiasedLocking` is enabled:
>
> void LIRGenerator::do_MonitorEnter(MonitorEnter* x) {
>   ...
>   // Need a scratch register for biased locking on arm
>   LIR_Opr scratch = LIR_OprFact::illegalOpr;
>   if(UseBiasedLocking) {
>     scratch = new_pointer_register();
>   } else {
>     scratch = atomicLockOpr(); // <--- actually illegalOpr
>   }
>
>   ...
>
>   monitor_enter(obj.result(), lock, hdr, scratch,
>                 x->monitor_no(), info_for_exception, info);
> }
>
> The way out is to use `tmp2`, which is the alias for `Rtemp` and always available.
>
> Additional testing:
>  - [x] Linux ARM32 `SyncOnValueBasedClassTest`, `-XX:+UseBiasedLocking`
>  - [x] Linux ARM32 `SyncOnValueBasedClassTest`, `-XX:-UseBiasedLocking`

Hi,
The change is good! I did not notice this Pull Request, I studied the case and came up with exactly the same solution.
Thanks for fixing this.
Boris

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

PR: https://git.openjdk.java.net/jdk/pull/2349
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8260899: ARM32: SyncOnValueBasedClassTest fails with assert(is_valid()) failed: invalid register

Aleksey Shipilev-5
On Fri, 5 Feb 2021 09:21:24 GMT, Boris Ulasevich <[hidden email]> wrote:

>> $ CONF=linux-arm-server-fastdebug make run-test TEST=runtime/Monitor/SyncOnValueBasedClassTest.java
>> ...
>>
>> #  Internal Error (/home/pi/jdk/src/hotspot/cpu/arm/register_arm.hpp:155), pid=3793, tid=3808
>> #  assert(is_valid()) failed: invalid register
>> #
>> # JRE version: OpenJDK Runtime Environment (17.0) (fastdebug build 17-internal+0-adhoc.pi.jdk)
>> # Java VM: OpenJDK Server VM (fastdebug 17-internal+0-adhoc.pi.jdk, compiled mode, emulated-client, g1 gc, linux-arm)
>> # Problematic frame:
>> # V  [libjvm.so+0xe1a6a8]  MacroAssembler::load_klass(RegisterImpl*, RegisterImpl*, AsmCondition)+0xa4
>>
>> Current CompileTask:
>> C1:    318    2   !b        java.lang.Class::desiredAssertionStatus (54 bytes)
>>
>> Stack: [0x72580000,0x72600000],  sp=0x725fe170,  free space=504k
>> Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
>> V  [libjvm.so+0xe1a6a8]  MacroAssembler::load_klass(RegisterImpl*, RegisterImpl*, AsmCondition)+0xa4
>> V  [libjvm.so+0x43b6b4]  C1_MacroAssembler::lock_object(RegisterImpl*, RegisterImpl*, RegisterImpl*, RegisterImpl*, Label&)+0xcf8
>> V  [libjvm.so+0x3d731c]  LIR_Assembler::emit_lock(LIR_OpLock*)+0x160
>>
>>
>> The problem is in this code:
>>
>>   if (DiagnoseSyncOnValueBasedClasses != 0) {
>>     load_klass(tmp1, obj); <--- asserts
>>     ldr_u32(tmp1, Address(tmp1, Klass::access_flags_offset()));
>>     tst(tmp1, JVM_ACC_IS_VALUE_BASED_CLASS);
>>     b(slow_case, ne);
>>   }
>>
>> `tmp1` is `noreg` when `!BiasedLocking`, because `c1_LIRGenerator_arm.cpp` provides it only when `UseBiasedLocking` is enabled:
>>
>> void LIRGenerator::do_MonitorEnter(MonitorEnter* x) {
>>   ...
>>   // Need a scratch register for biased locking on arm
>>   LIR_Opr scratch = LIR_OprFact::illegalOpr;
>>   if(UseBiasedLocking) {
>>     scratch = new_pointer_register();
>>   } else {
>>     scratch = atomicLockOpr(); // <--- actually illegalOpr
>>   }
>>
>>   ...
>>
>>   monitor_enter(obj.result(), lock, hdr, scratch,
>>                 x->monitor_no(), info_for_exception, info);
>> }
>>
>> The way out is to use `tmp2`, which is the alias for `Rtemp` and always available.
>>
>> Additional testing:
>>  - [x] Linux ARM32 `SyncOnValueBasedClassTest`, `-XX:+UseBiasedLocking`
>>  - [x] Linux ARM32 `SyncOnValueBasedClassTest`, `-XX:-UseBiasedLocking`
>
> Hi,
> The change is good! I did not notice this Pull Request, I studied the case and came up with exactly the same solution.
> Thanks for fixing this.
> Boris

Thanks! I need a formal Reviewer to ack this :)

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

PR: https://git.openjdk.java.net/jdk/pull/2349
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8260899: ARM32: SyncOnValueBasedClassTest fails with assert(is_valid()) failed: invalid register

Dmitry Samersoff-4
In reply to this post by Boris Ulasevich-2
On Tue, 2 Feb 2021 09:24:20 GMT, Aleksey Shipilev <[hidden email]> wrote:

> $ CONF=linux-arm-server-fastdebug make run-test TEST=runtime/Monitor/SyncOnValueBasedClassTest.java
> ...
>
> #  Internal Error (/home/pi/jdk/src/hotspot/cpu/arm/register_arm.hpp:155), pid=3793, tid=3808
> #  assert(is_valid()) failed: invalid register
> #
> # JRE version: OpenJDK Runtime Environment (17.0) (fastdebug build 17-internal+0-adhoc.pi.jdk)
> # Java VM: OpenJDK Server VM (fastdebug 17-internal+0-adhoc.pi.jdk, compiled mode, emulated-client, g1 gc, linux-arm)
> # Problematic frame:
> # V  [libjvm.so+0xe1a6a8]  MacroAssembler::load_klass(RegisterImpl*, RegisterImpl*, AsmCondition)+0xa4
>
> Current CompileTask:
> C1:    318    2   !b        java.lang.Class::desiredAssertionStatus (54 bytes)
>
> Stack: [0x72580000,0x72600000],  sp=0x725fe170,  free space=504k
> Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
> V  [libjvm.so+0xe1a6a8]  MacroAssembler::load_klass(RegisterImpl*, RegisterImpl*, AsmCondition)+0xa4
> V  [libjvm.so+0x43b6b4]  C1_MacroAssembler::lock_object(RegisterImpl*, RegisterImpl*, RegisterImpl*, RegisterImpl*, Label&)+0xcf8
> V  [libjvm.so+0x3d731c]  LIR_Assembler::emit_lock(LIR_OpLock*)+0x160
>
>
> The problem is in this code:
>
>   if (DiagnoseSyncOnValueBasedClasses != 0) {
>     load_klass(tmp1, obj); <--- asserts
>     ldr_u32(tmp1, Address(tmp1, Klass::access_flags_offset()));
>     tst(tmp1, JVM_ACC_IS_VALUE_BASED_CLASS);
>     b(slow_case, ne);
>   }
>
> `tmp1` is `noreg` when `!BiasedLocking`, because `c1_LIRGenerator_arm.cpp` provides it only when `UseBiasedLocking` is enabled:
>
> void LIRGenerator::do_MonitorEnter(MonitorEnter* x) {
>   ...
>   // Need a scratch register for biased locking on arm
>   LIR_Opr scratch = LIR_OprFact::illegalOpr;
>   if(UseBiasedLocking) {
>     scratch = new_pointer_register();
>   } else {
>     scratch = atomicLockOpr(); // <--- actually illegalOpr
>   }
>
>   ...
>
>   monitor_enter(obj.result(), lock, hdr, scratch,
>                 x->monitor_no(), info_for_exception, info);
> }
>
> The way out is to use `tmp2`, which is the alias for `Rtemp` and always available.
>
> Additional testing:
>  - [x] Linux ARM32 `SyncOnValueBasedClassTest`, `-XX:+UseBiasedLocking`
>  - [x] Linux ARM32 `SyncOnValueBasedClassTest`, `-XX:-UseBiasedLocking`

Marked as reviewed by dsamersoff (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/2349
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8260899: ARM32: SyncOnValueBasedClassTest fails with assert(is_valid()) failed: invalid register

Dmitry Samersoff-4
In reply to this post by Aleksey Shipilev-5
On Fri, 5 Feb 2021 09:48:54 GMT, Aleksey Shipilev <[hidden email]> wrote:

>> Hi,
>> The change is good! I did not notice this Pull Request, I studied the case and came up with exactly the same solution.
>> Thanks for fixing this.
>> Boris
>
> Thanks! I need a formal Reviewer to ack this :)

Looks good to me (R)

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

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