Thanks for taking this on. It looks good, except for one thing. The intrinsic does not need to be an instance method, and doing so creates an undesirable coupling between the JVM and JDK. Specifically, the JDK should not need to know about subkeyH and state fields. The Java code should pass those as plain (array long[2]) arguments to the intrinsic method processBlocks, which should be adjusted to be static. The domain check routine should be adjusted to be static also.
On my wish list for the future (but not now) is even less coupling with the JVM. The loop code for processBlocks should be written in Java, with various intrinsics (xmulx*) for dealing with single operations on 128-bit values (stored in long[2] boxes and 64-bit registers). The Unsafe misaligned access routines could help simplify this also, if the coding were done in Java. This is not too hard to express in Java and compile to excellent code. There will be a little extra awkwardness working with 64x2-vectors in a way that will compile naturally to a range of ALUs (both 64- and 128-bit). If we get it right we can reduce the amount of assembly code in the JVM and get even more timely access to new data-processing instructions. Would you please file a followup bug (low pri. for now) to track this, at least for GHASH and other crypto loops? — John On Jun 15, 2015, at 4:26 PM, Anthony Scarpino <anthony.scarp...@oracle.com> wrote: > > Hi other reviewers, > > I'm hoping someone else can review this. > > Tony > > On 06/12/2015 03:16 PM, Vladimir Kozlov wrote: >> This implementation looks good to me. You need second review since >> changes are big. >> >> Thanks, >> Vladimir >> >> On 6/11/15 11:01 AM, Anthony Scarpino wrote: >>> Now that JEP 246 is targeted, I can proceed a with the GHASH changes I >>> had reviewed back in February. >>> >>> There are two changes from the previous review. One is the separate >>> method for the range checking in jdk:GHASH.java in update(). The other >>> is disabling UseGHASHIntrinsics for arch64 in hotspot. >>> >>> http://cr.openjdk.java.net/~ascarpino/8073108/hotspot/webrev.03/ >>> http://cr.openjdk.java.net/~ascarpino/8073108/jdk/webrev.03/ >>> >>> thanks >>> >>> Tony >>> >
