RFR: 8073108: GHASH Intrinsics

[John Rose]

Cool stuff.  I'm glad to see SPARC xmulx getting some play.

This is not a review, but I have a small and a big comment.

You don't need the argument vmIntrinsics::ID id unless you are going to do something with it, such as expand one of a family of intrinsics covered by the same LCK::inline* routine.

You probably don't have enough parameter verification on the inputs to processBlocks.  A robust range check for a subsequence indexing operation requires three comparison operations, not just one.  The Java code uses getLong which includes additional range checks, but your intrinsic replacement code might not do that correctly; in any case, it's better to have all the subsequence range checks in one place, and in Java code.  (Oh, and watch out for 32-bit integer overflow:  That causes surprises sometimes, when a value that is too large wraps to negative, and then can seem to be in range relative to a <= check.)

In fact, I don't believe the existing parameter verification is very helpful at all:

   if (inLen - inOfs > in.length)  throw...

This appears to allow a ridiculously large inLen as long as inOfs is also ridiculously large.  All of the "heavy lifting" is done by intrinisic array range checks triggered by getLong, and those are the checks that the assembly code does *not* do.

— John

P.S.  We should have a library API to do these parameter checks.  See:
  https://bugs.openjdk.java.net/browse/JDK-8042997#comment-13610181

On Feb 16, 2015, at 1:11 PM, Anthony Scarpino <anthony.scarpino at oracle.com> wrote:
> 
> Hi,
> 
> I'm requesting a code review to intrinsify the GHASH operations for both x86 and SPARC platforms.  This greatly increases performance over software for AES/GCM crypto operations, details are in the bug.
> 
> The review is for two repos, hotspot and jdk:
> 
> http://cr.openjdk.java.net/~ascarpino/8073108/hotspot/webrev/
> 
> http://cr.openjdk.java.net/~ascarpino/8073108/jdk/webrev/
> 
> thanks
> 
> Tony