RFR(XS): 8248219: aarch64: missing memory barrier in fast_storefield and fast_accessfield

[Yangfei (Felix)]

Ping...

Is it OK to push?

Thanks,
Felix

> -----Original Message-----
> From: Yangfei (Felix)
> Sent: Sunday, June 28, 2020 8:32 PM
> To: 'Andrew Haley' <aph at redhat.com>; hotspot-runtime-
> dev at openjdk.java.net
> Cc: aarch64-port-dev at openjdk.java.net
> Subject: RE: RFR(XS): 8248219: aarch64: missing memory barrier in
> fast_storefield and fast_accessfield
> 
> Hi Andrew,
> 
>   Sorry for the late reply.  It's Dragon Boat Festival here in China.
> 
> > -----Original Message-----
> > From: Andrew Haley [mailto:aph at redhat.com]
> > Sent: Wednesday, June 24, 2020 6:45 PM
> > To: Yangfei (Felix) <felix.yang at huawei.com>; hotspot-runtime-
> > dev at openjdk.java.net
> > Cc: aarch64-port-dev at openjdk.java.net
> > Subject: Re: RFR(XS): 8248219: aarch64: missing memory barrier in
> > fast_storefield and fast_accessfield
> >
> > On 24/06/2020 10:38, Yangfei (Felix) wrote:
> > >   Suggestions?
> >
> > Great catch!
> 
> Thanks for the quick review :-)
> 
> > Thanks for that, I completely agree. Please benchmark the two and
> > unless there is an advantage for the address dependency we'll go with
> > LoadLoad.  It
> 
> I use a simple test [1] to exercise the read & write object field operation and
> run it in -Xint mode.
> Test result on our aarch64 platform show no advantage for the address
> dependency way.
> So I think I am OK to go with the LoadLoad way.
> 
> Webrev: http://cr.openjdk.java.net/~fyang/8248219/webrev.00/
> 
> > looks like all of the ConstantPoolCacheEntry::set methods use
> > Atomic::release_store, so everything should be fine there.
> >
> > Did you also look to see if we need similar memory barriers elsewhere?
> 
> Yes, I checked and only saw these two places.
> 
> > We're going to need backports for all extant OpenJDK versions. Please
> > let us know if you can handle that too.
> 
> Well, I think at least I can handle jdk8u-shenandoah, jdk11u and jdk/jdk.
> 
> Felix
> 
> [1]
> class TestCP {
> 
>   public int num;
> 
>   void run(int reps) {
>     for (int i = 0; i < reps; i++) {
>       num += i;
>     }
>   }
> 
>   public static void main(String[] args) throws Exception {
>     int reps = Integer.parseInt(System.getProperty("repos"));
>     TestCP t = new TestCP();
> 
>     Long startTime = System.nanoTime();
>     t.run(reps);
>     Long endTime = System.nanoTime();
>     System.out.println(endTime - startTime);
>   }
> }