Unsafe.{get,put}-X-Unaligned; Efficient array comparison intrinsics

[Paul Sandoz]

On Mar 2, 2015, at 8:30 PM, Andrew Haley <aph at redhat.com> wrote:

> On 02/25/2015 04:43 PM, Andrew Haley wrote:
>> On 02/24/2015 11:18 PM, John Rose wrote:
>>> My bottom line:  I think we should use the internal HotSpot
>>> API bytes.hpp by surfacing relevant parts of it up into Unsafe.
> 
> I have done this except that I have used the Java typenames for the
> methods.
> 
>>> Looking at the various platform code for bytes.hpp, I suppose
>>> one could argue that there are just two cases of interest:
>>> A single instruction (x86/arm) and a complicated 4-way
>>> switch (sparc/ppc). 
> 
> And 32-bit ARM which is usually little-endian with no unaligned
> support.
> 
>>> Distinct big and little endian access modes are also available
>>> on some machines, such as SPARC (ASI_LITTLE, etc.).
>>> But I do *not* believe that this capability should be surfaced
>>> as distinct intrinsics in Unsafe.  Many cpus and source bases
>>> deal with endian-matching simply by adjoining a separate
>>> "byte swap" operation to the memory access.  In Java,
>>> this is already an intrinsic, {Long,Integer,...}.reverseBytes.
>>> And SPARC already optimizes some compositions of
>>> byte reversal and memory access to its special ASI_LITTLE
>>> instructions.
>>> 
>>> My second bottom line:  Don't multiply endian options.
>>> Use reverseBytes calls instead.
> 
> I have done this as much as is possible, but methods which assemble
> and split sub-words are necessarily endian-dependent.  I have
> separated the native big- and little-endian code into two classes,
> only one of which will ever be loaded into a system.
> 
>>> Suggestion:  Have getIntUnaligned take an optional boolean
>>> parameter, which is "bigEndian" (since that's relatively exceptional).
>>> An extra line of code can conditionally swap the bytes, taking
>>> both the flag and the platform into account.  Default value of the
>>> boolean is whatever is natural to the platform.  If you specifically
>>> want Java's big-endian order, you specify true, etc.
> 
> I've done this in Java.  I tried some HotSpot intrinsic code to handle
> the "bigEndian" parameter but foundered when I noticed that
> Op_ReverseBytesXX nodes are optional, so an intrinsic might either be
> emitted as an instruction or a call to a native method.  This was all
> too messy so I reverted to doing it in Java; the code quality does not
> seem to suffer as a result of this.  (Another possibility is to define
> intrinsics which would only be used if Op_ReverseBytesXX nodes were
> supported, but I don't know that such an intrinsic would buy us
> anything.)
> 
> It wasn't clear to me whether you might also want set-XX-Unaligned
> methods with specific endianness, so I didn't write any.
> 
> I have not written any Unsafe support for floating-point types.  It's
> not clear to me that it is needed.  Perhaps the caller should convert
> floating-point types as necessary; what do you think?
> 
> There are several places in the JDK where we have special cases for
> alignment, endianness, and arrays and some could benefit from use of
> these new methods, but except for HeapByteBuffers I haven't changed
> anything.
> 
> I am aware that the code is uncommented.  I will fix that once we
> agree about what to do next.
> 
> http://cr.openjdk.java.net/~aph/unaligned.hotspot.1/
> http://cr.openjdk.java.net/~aph/unaligned.jdk.2/
> 

This is looking good.


The flag UseUnalignedAccesses feels a little awkward. IIUC it seems to be acting as two things, a flag signalling an unaligned architecture and a developer option to disable/enable unaligned intrinsics.  Should this flag even be allowed to be set to true on aligned architectures?

Perhaps we need to separate out into two constants? one exposed via Unsafe.unalignedAccess, and then a developer flag UseUnalignedAccessesIntrinsics (true by default if unaligned architecture) to disable intrinsics for testing purposes.


My inclination is to remove the get*Unaligned(..., boolean bigEndian) methods and thereby consistently use Bits.swap in the heap buffer. A similar pattern applies for float/double conversion.


It might be useful to have unaligned access methods for the single-register addressing mode methods. Using those would clean up logic in MappedByteBuffer and perhaps similar twiddling methods in Bits could be removed, thus consolidating all such logic within Unsafe?


Paul.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150304/29d11c3d/signature.asc>