RFR (M) 8157726: VarHandles/Unsafe should support sub-word atomic ops [RESEND]

[Paul Sandoz]

Hi,

This looks good to me.

Nice work, and done in a short time-frame.

I like the fact that minimal changes were required to the VH implementations and tests, since it just a tweak to the relevant templates.

Paul.

> On 27 May 2016, at 15:52, Aleksey Shipilev <aleksey.shipilev at oracle.com> wrote:
> 
> (Please note this work is covered with FC exception)
> 
> 
> (If you receive a previous version without [RESEND], please ignore)
> 
> Hi,
> 
> 
> 
> Please review the VarHandles/Unsafe support for sub-word atomic ops:
> 
>  https://bugs.openjdk.java.net/browse/JDK-8157726
> 
> 
> 
> Webrevs:
> 
>  http://cr.openjdk.java.net/~shade/8157726/webrev.hs.03/
> 
>  http://cr.openjdk.java.net/~shade/8157726/webrev.jdk.03/
> 
> 
> 
> It *looks* like a lot of changes, but the bulk of changes are in tests,
> and the rest of product code is mostly duplicating the existing cases
> for the new code.
> 
> 
> 
> Tour of the changes:
> 
> 
> 
> Unsafe.java:
> 
> 
> 
>  *) Implements the baseline implementation for subword CASes, which
> does the CAS loop within an aligned integer word. While we could
> potentially detect 64-bit platform and do aligned long word instead,
> this complicates implementation for no reason. 32-bit CASes are ubiquitous.
> 
> 
> 
>  *) Provides the entry points for VarHandles implementation. While
> Unsafe provides the implementation for byte/short atomic ops only, it
> also provides the boolean/char versions that are trivially implementable
> with byte/short ops. While we could instead make VarHandles do this
> conversion, it proved harder to do this in VarHandle templates, rather
> than in Unsafe.
> 
> 
> 
>  *) Consistently uses weakCompareAndSetVolatile for CAS loops, in both
> new and old code, which is important for a fallback implementation to
> work well on non-x86 platforms.
> 
> 
> 
> VarHandles*:
> 
> 
> 
>  *) Now mentions subword atomic ops are available in method spec. We
> have submitted CCC for this.
> 
> 
>  *) VarHandle templates are slightly modified to do narrowing
> conversions for shorter-than-int types. This keeps us from putting the
> entire family of addAndGet methods to Unsafe.
> 
> 
> 
>  *) Test templates are changed to accommodate new code shapes, and also
> change the bit patterns we use to detect endianness issues.
> 
> 
> 
> ----- everything above does functionally complete subword ops ----
> --
> ------- everything below is the key performance optimization -------
> 
> 
> 
> Platform-independent changes:
> 
> 
> 
>  *) vmSymbols.hpp, library_call.cpp and other changes: the usual set of
> scaffolding for Unsafe intrinsics. We reuse most of the infrastructure,
> adding new nodes/cases in the existing code.
> 
> 
> 
>  *) Unsafe tests are are changed to accommodate new code shapes, and
> also change the bit patterns we use to detect endianness issues.
> 
> 
> 
> x86-specific changes:
> 
> 
> 
>  *) Full set of new x86_{32|64} intrinsics are provided in .ad files.
> Supporting 8-byte and 16-byte ops required introducing new operations in
> x86 assembler. These are tested by new compiler/unsafe tests.
> 
> 
> 
> 
> 
> Performance data justifies the change, and highlights pitfalls:
> 
>  http://cr.openjdk.java.net/~shade/8157726/notes.txt
> 
> 
> 
> 
> Testing: eyeballing x86 assembly; targeted benchmarks;
> {java/lang/invoke/VarHandles, compiler/unsafe} tests on x86_{32|64},
> POWER; RBT hs-comp-tier0.
> 
> 
> 
> Thanks,
> 
> -Aleksey
>