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

dean.long at oracle.com dean.long at oracle.com
Mon Dec 17 18:52:11 UTC 2018


This has been pushed now:

http://hg.openjdk.java.net/jdk/jdk12/rev/5da72d7e0e80

dl

On 12/17/18 12:26 AM, Nick Gasson (Arm Technology China) wrote:
> 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
>>>



More information about the aarch32-port-dev mailing list