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

[Vladimir Kozlov]

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