[aarch64-port-dev ] aarch64: C2 fast lock/unlock issues

[Hui Shi]

Thanks Vladimir!

Regards
Shi Hui

On 22 August 2015 at 05:24, Vladimir Kozlov <vladimir.kozlov at oracle.com>
wrote:

> Thank you for report and suggested fixes.
>
> CC to aarch64 port developers.
>
> Thanks,
> Vladimir
>
> On 8/21/15 5:21 AM, Hui Shi wrote:
>
>> Hi JIT members,
>> Attached fast_lock.patch fixes issues in fast lock/unlock on aarch64
>> platform (in both aarch64-jdk8 and jdk9/hs-comp/hotspot). Could someone
>> help comments, review or sponsor?
>> A small test case and  PrintAssembly log with/without fix are also
>> attached for reference.
>>
>> To reproduce this issue on aarch64, command line is "java
>> -XX:-TieredCompilation -XX:+UnlockDiagnosticVMOptions
>> -XX:-BackgroundCompilation -XX:CompileCommand="compileonly,TestSync.f*"
>> -XX:+PrintAssembly  TestSync"
>> There are three Issues in aarch64 fast lock/unlock:
>> *1. Duplicated biased lock checking*
>> When option UseBiasedLocking and UseOptoBiasInlining are both true, it
>> doesn't need emit biased_locking_enter in aarch64_enc_fast_lock. This is
>> redundant as biased locking enter check is already inlined in
>> PhaseMacroExpand::expand_lock_node. Checking assembly code in orig.asm
>>
>> [Inlined biased lock check in PhaseMacroExpand::expand_lock_node]
>>    0x000003ff88320d94: str       x1, [sp]
>>    0x000003ff88320d98: ldr       x10, [x1]
>>    0x000003ff88320d9c: and       x11, x10, #0x7
>>    0x000003ff88320da0: cmp       x11, #0x5
>>    0x000003ff88320da4: b.ne <http://b.ne>      0x000003ff88320e18
>> [Biased lock check expanded in aarch64_enc_fast_lock]
>>    0x000003ff88320e18: add       x12, sp, #0x10
>>    0x000003ff88320e1c: ldr       x10, [x1]
>>    0x000003ff88320e20: and       x11, x12, #0x7
>>    0x000003ff88320e24: cmp       x11, #0x5
>>    0x000003ff88320e28: b.ne <http://b.ne>      0x000003ff88320eec
>> *2. Incorrect parameter used in biased_locking_enter in
>> aarch64_enc_fast_lock*
>> Checking above code [Biased lock check expanded in
>> aarch64_enc_fast_lock], x12 is the box register and holding the address
>> of the lock record on stack. However it is mis-used as mark word in
>> biased lock checking here. As a result, biased pattern check always
>> fails because stack pointer is 8 bytes align and x11 must be zero.
>> Current implementation in aarch64_enc_fast_lock.
>> /biased_locking_enter(disp_hdr, oop, box, tmp, true, cont);/
>> Which should be
>> /biased_locking_enter(box, oop, disp_hdr, tmp, true, cont);  //swap
>> disp_hdr and box register, disp_hdr is already loaded with object mark
>> word/
>> This issue might cause problem when running with option
>> -XX:-UseOptoBiasInlining in following scenario, let’s check above code
>> in [Biased lock check expanded in aarch64_enc_fast_lock], x12 is box and
>> x10 is disp_hdr.
>> 1. Suppose object’s mark word (loaded into register x10) is in biased
>> mode, with content “[biased_thread |epoch|age| 101]” and biased_thread
>> is executing its synchronized block.
>> 2. Another thread tries to acquire the same lock. Firstly, it performs
>> biased pattern check and fails, because “mark word” register used here
>> is X12 (correct register should be x10).
>> 3. As x12 is not “biased” (least three significant bits of SP + 0x10
>> would never be 101),  execution goes to thin lock CAS acquire code
>> instead of biased lock revoke/rebias code.
>> 4. Thin lock CAS acquire will succeed because x10’s least two
>> significant bit is 01 (thin lock CAS code uses disp_hdr (x10) as mark
>> word). Two threads acquire same lock at same time and this is incorrect
>> behavior.
>>
>> *3. Inflate monitor code has typo in aarch64_enc_fast_lock*
>> Inflated lock test is generated under condition (EmitSync & 0x02), while
>> generating inflated lock fast path under condition "if ((EmitSync &
>> 0x02) == 0))". At both location, they should be "if ((EmitSync & 0x02)
>> == 0) ". In orig.asm, no instruction branches to inflated lock acquire
>> fast path at 0x000003ff88320f24.
>> Issue #1 and #3 does not impact correctness, they introduce redundant
>> code (double biased lock check) and skip inflated lock fast path check
>> (_owner is null case).
>> Fix is in aarch64_enc_fast_lock/aarch64_enc_fast_unlock, this will not
>> impact C1 and interpreter. Attached patch includes:
>> 1. Disable generating biased lock handle code in fast_lock/fast_unlock
>> when UseOptoBiasInlining is true.
>> 2. Adjust biased_locking_enter’s actual parameters, swap disp_hdr and
>> box register.
>> 3. Fix typo in inflated monitor handling.
>>
>> Regards
>> Shi Hui
>>
>