RFR: 8214512: Jtreg test compiler/c2/Test8062950.java fails on ARM32

Boris Ulasevich boris.ulasevich at bell-sw.com
Wed Dec 12 11:41:55 UTC 2018


The change is Ok for me.

 > and let the runtime handle the failure case.

One minor comment. It would be good to add inline comment to 
MacroAssembler::fast_lock bottom. Compare with other ports:

aarch: __ bind(cont);
aarch: // flag == EQ indicates success
aarch: // flag == NE indicates failure

x86:   // At DONE_LABEL the icc ZFlag is set as follows ...
x86:   // Fast_Unlock uses the same protocol.
x86:   // ZFlag == 1 -> Success
x86:   // ZFlag == 0 -> Failure - force control through the slow-path

best regards,

On 03.12.2018 9:06, Nick Gasson wrote:
> Hi,
> Could someone please help me review this fix for a test failure on ARM32:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214512
> Webrev: http://cr.openjdk.java.net/~njian/8214512/webrev.0/
> This fixes two assertion failures related to biased locking when C2's
> bias inlining feature is disabled (-XX:-OptoBiasInlining):
> MacroAssembler::atomic_cas_bool takes three register arguments plus a
> temporary register for use in the CAS loop. If the caller passes
> `noreg' as this temporary register (which happens in the
> !OptoBiasInlining call path from MacroAssembler::fast_lock) then
> MacroAssembler::atomic_cas_bool will default to using LR as a
> temporary. But it's possible with C2 that LR is one of the other three
> arguments which then triggers an assert_different_registers assertion
> failure. Fix this by supplying an additional scratch register to
> MacroAssembler::fast_lock if !OptoBiasInlining.
> In the !OptoBiasInlining case MacroAssembler::fast_lock calls
> MacroAssembler::biased_locking_enter to handle the case where the mark
> word contains the biased lock pattern which fast_lock wouldn't
> otherwise see if OptoBiasInlining was true. However in the case that
> we fail to acquire the biased lock this falls through to label
> `failed' and runs the normal fast_lock code that implicitly assumes
> the mark word does not have the bias pattern. This means we can end up
> with a BasicLock where _displaced_header contains the biased lock
> pattern which is an illegal state and subsequently triggers an
> assertion failure in ObjectSynchronizer::fast_exit.
> The right thing to do here is branch to `done' whenever the bias lock
> pattern is present and let the runtime handle the failure case.  Also
> edited the comment describing MacroAssembler::biased_locking_enter to
> more accurately describe what it does (the comment about `slow_case'
> being optional is wrong).
> Jtreg test compiler/c2/Test8062950.java now passes on ARM32 with this
> patch.
> Thanks,
> Nick

More information about the hotspot-dev mailing list