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

[Nick Gasson (Arm Technology China)]

Hi Dean,

> Would you like to add comments as suggested by Boris?  I think 
> it's a good idea.

Sure, I've updated the webrev here:

http://cr.openjdk.java.net/~njian/8214512/webrev.1/

Thanks,
Nick


On 17/12/2018 12:59, dean.long at oracle.com wrote:
> Thanks.  Would you like to add comments as suggested by Boris?  I think 
> it's a good idea.
> 
> dl
> 
> On 12/16/18 6:00 PM, Nick Gasson (Arm Technology China) wrote:
>>
>> Hi Dean,
>>
>> Sorry for the delay. This is submitted on behalf of ARM Ltd.
>>
>> Thanks,
>>
>> Nick
>>
>> *From:* dean.long at oracle.com <dean.long at oracle.com>
>> *Sent:* 13 December 2018 12:01
>> *To:* Nick Gasson (Arm Technology China) <Nick.Gasson at arm.com>; 
>> hotspot-dev at openjdk.java.net; Boris Ulasevich 
>> <boris.ulasevich at bell-sw.com>
>> *Cc:* hotspot-compiler-dev at openjdk.java.net compiler 
>> <hotspot-compiler-dev at openjdk.java.net>; nd <nd at arm.com>; 
>> aarch32-port-dev at openjdk.java.net
>> *Subject:* Re: RFR: 8214512: Jtreg test compiler/c2/Test8062950.java 
>> fails on ARM32
>>
>> Nick, can you please confirm that you are covered under the
>>
>> · ARM Ltd. - OpenJDK, MySQL
>>
>>
>> OCA agreement?  I'm guessing Arm Technology China is under ARM Ltd., 
>> judging
>> by your email domain, but please confirm.
>>
>> dl
>>
>> On 12/11/18 5:30 PM, dean.long at oracle.com 
>> <mailto:dean.long at oracle.com> wrote:
>>
>>     I can help you commit it, but first I think another review would
>>     be good.
>>
>>     dl
>>
>>     On 12/11/18 5:15 PM, Nick Gasson (Arm Technology China) wrote:
>>
>>             Hi Nick.  This change looks good to me.
>>
>>         Thanks Dean. Could you help me to commit this patch if it
>>         doesn't need other reviews?
>>
>>         Nick
>>
>>
>>
>>             -----Original Message-----
>>             From: dean.long at oracle.com <mailto:dean.long at oracle.com>
>>             <dean.long at oracle.com> <mailto:dean.long at oracle.com>
>>             Sent: 12 December 2018 06:05
>>             To: Nick Gasson (Arm Technology China)
>>             <Nick.Gasson at arm.com> <mailto:Nick.Gasson at arm.com>; hotspot-
>>             dev at openjdk.java.net <mailto:dev at openjdk.java.net>; Boris
>>             Ulasevich <boris.ulasevich at bell-sw.com>
>>             <mailto:boris.ulasevich at bell-sw.com>
>>             Cc: nd <nd at arm.com> <mailto:nd at arm.com>;
>>             hotspot-compiler-dev at openjdk.java.net
>>             <mailto:hotspot-compiler-dev at openjdk.java.net> compiler
>>             <hotspot-compiler-dev at openjdk.java.net>
>>             <mailto:hotspot-compiler-dev at openjdk.java.net>; aarch32-port-
>>             dev at openjdk.java.net <mailto:dev at openjdk.java.net>
>>             Subject: Re: RFR: 8214512: Jtreg test
>>             compiler/c2/Test8062950.java fails on
>>             ARM32
>>
>>             Hi Nick.  This change looks good to me.
>>
>>             dl
>>
>>             On 12/2/18 10:06 PM, 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
>>
>