RFR: 8319947: Recursive lightweight locking: s390x implementation

[Amit Kumar]

On Wed, 22 May 2024 07:14:59 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> The current code is fine, but that comment made me wonder why preserving the original top value was important. My think was that you could only change the assert snippet as follows:
>> 
>> #ifdef ASSERT
>>     NearLabel check_done;
>> +   NearLabel loop;
>> +   z_lgf(top, Address(Z_thread, JavaThread::lock_stack_top_offset()));
>> +   bind(loop);
>>     z_aghi(top, -oopSize);
>>     compareU32_and_branch(top, in_bytes(JavaThread::lock_stack_base_offset()),
>>                           bcondLow, check_done);
>>     z_cg(obj, Address(Z_thread, top));
>> -   z_brne(inflated);
>> +   z_brne(loop);
>>     stop("Fast Unlock lock on stack");
>>     bind(check_done);
>> #endif // ASSERT
>> 
>> 
>> then remove the comment and use either whatever register.
>
> I still do not think I understand what ` if we load top there then it could result into infinite loop` is referring to.

Initial version of my code change was like this: 

diff --git a/src/hotspot/cpu/s390/macroAssembler_s390.cpp b/src/hotspot/cpu/s390/macroAssembler_s390.cpp
index 5a77e0d49f2..8fd3e241ca8 100644
--- a/src/hotspot/cpu/s390/macroAssembler_s390.cpp
+++ b/src/hotspot/cpu/s390/macroAssembler_s390.cpp
@@ -6064,7 +6064,7 @@ void MacroAssembler::compiler_fast_unlock_lightweight_object(Register obj, Regis
 #endif // ASSERT
 
     bind(inflated);
-
+    z_lgf(top, Address(Z_thread, JavaThread::lock_stack_top_offset()));
 #ifdef ASSERT
     NearLabel check_done;
     z_aghi(top, -oopSize);


that's why added that comment there. But personally I liked the suggestion you gave. I'll update the code.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18878#discussion_r1609545336