RFR(xs): 8211387: [Zero] atomic_copy64: Use ldrexd for atomic reads on ARMv7

[Severin Gehwolf]

Hi David,

On Fri, 2018-10-05 at 07:28 +1000, David Holmes wrote:
> Hi Severin,
> 
> On 4/10/2018 11:03 PM, Severin Gehwolf wrote:
> > Hi,
> > 
> > Please review this forward port for Zero coming from JDK 7 (where Zero
> > is maintained in icedtea) to JDK 12. The update is to use ldrexd
> > instructions on ARMv7 CPUs for atomic_copy64() in Zero:
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8211387
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8211387/webrev.01/
> 
> Not sure how zero normally operates but I would have expected to see 
> something more dynamic (as used in the rest of the VM: 
> VM_version::supports_ldrexd()) rather than hardwiring for a v7_A build.

Unfortunately, this isn't as easily done for Zero. It might be running
on some arbitrary architecture. Not a specific one which supports
different features, like arm. We don't initialize VM_version of Zero
with any sort of achitecture specific info for that reason. Since
ldrexd is available for ARMv7 and up guarding code being generated at
compile time that way seems reasonable to me.

> Also I thought that if ldrex was not paired with a strex then you should 
> at least issue clrex - as done in generate_atomic_load_long() in 
> stubGenerator_arm.cpp

I'll defer to Andrew Haley for this. FWIW, the arm version snippet from
generate_atomic_load_long() looks like this:

    if (!os::is_MP()) {
      __ ldmia(src, RegisterSet(result_lo, result_hi));
      __ bx(LR);
    } else if (VM_Version::supports_ldrexd()) {
      __ ldrexd(result_lo, Address(src));
      __ clrex(); // FIXME: safe to remove?
      __ bx(LR);
    } else {
      __ stop("Atomic load(jlong) unsupported on this platform");
      __ bx(LR);
    }

Note the FIXME. I wonder what's the best course of action here. a)
remove clrex() in the arm port or b) add clrex to the Zero atomic
assembly too and remove the FIXME. Something else?

Thanks,
Severin

> Cheers,
> David
> 
> > Testing: Zero bootcycle-images build on armv7 where this was satisfied:
> > 
> > $ gcc -dM -E - < /dev/null | grep __ARM_ARCH_7A__
> > #define __ARM_ARCH_7A__ 1
> > 
> > This change was contributed by Andrew Haley.
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin
> >