(8u) RFR: 8134869: AARCH64: GHASH intrinsic is not optimal

[Hohensee, Paul]

Lgtm. :)

-----Original Message-----
From: "Liu, Xin" <xxinliu at amazon.com>
Date: Wednesday, August 25, 2021 at 10:31 AM
To: "Hohensee, Paul" <hohensee at amazon.com>, Andrew Haley <aph-open at littlepinkcloud.com>, "jdk8u-dev at openjdk.java.net" <jdk8u-dev at openjdk.java.net>
Subject: Re: (8u) RFR: 8134869: AARCH64: GHASH intrinsic is not optimal

hi, Paul,

Thanks for reviewing this.

I didn't leave out that format block intentionally. I figure out why
webrev.ksh doesn't display it in file changes. I have to add '-b' to
generates whitespace diffs.

I've updated webrev to reflect all changes. Sorry for the confusion.
https://cr.openjdk.java.net/~xliu/8134869/


thanks,
--lx


On 8/25/21 7:47 AM, Hohensee, Paul wrote:
> I've changed the subject to reflect the actual JBS issue, vis
> 
> https://bugs.openjdk.java.net/browse/JDK-8134869
> 
> A (trivial) issue is that in assembler_aarch64.hpp, you left out a format (non-semantic) change to the macro definition containing " void NAME(FloatRegister Vd, SIMD_Arrangement T, FloatRegister Vn, unsigned registers, FloatRegister Vm)". Future backports may/will be easier if you include it.
> 
> Otherwise, lgtm.
> 
> Thanks,
> Paul
> 
> -----Original Message-----
> From: jdk8u-dev <jdk8u-dev-retn at openjdk.java.net> on behalf of "Liu, Xin" <xxinliu at amazon.com>
> Date: Monday, August 23, 2021 at 11:35 PM
> To: Andrew Haley <aph-open at littlepinkcloud.com>, "jdk8u-dev at openjdk.java.net" <jdk8u-dev at openjdk.java.net>
> Subject: Re: (8u) RFR: 8131062: aarch64: add support for GHASH acceleration
> 
> Hi, Andrew,
> 
> Thank you for taking time to review it.
> 
> Our motivation is mainly performance. Some customers would like to stay
> in jdk8u for some reasons. They consider to migrate from x86_64 to
> armv8. They witness this performance disparity. This patch ensures that
> jdk8u GHASH is a level playing field.
> 
> To be honest, I haven't understood the timing attack. I think the prior
> GHash function handles each block in constant time too. This patch makes
> it faster but it doesn't change this property.
> 
>> However, this is an old version of the GHASH intrinsic. You really want
>> 8134869: AARCH64: GHASH intrinsic is not optimal.
>> https://bugs.openjdk.java.net/browse/JDK-8134869
>>
>> It'll need a jdk8u-fix-request tag.
> 
> Thanks you for the pointer. I backport this as well. As you said, this
> revision refactors code with comments and becomes more idiomatic in
> armv8. Not only I verify its correctness, I also measure performance
> using microbenchmarks. I observe extra ~20% performance on top of
> JDK-8131062.
> 
> This patch can't apply cleanly, but it only needs some cosmetic changes.
> Could you take a look at it?
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8131062
> webrev: https://cr.openjdk.java.net/~xliu/8134869/webrev/
> 
> 
> thanks,
> --lx
> 
> 
> 
> On 8/21/21 2:44 AM, Andrew Haley wrote:
>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>>
>>
>>
>> On 8/16/21 6:59 PM, Liu, Xin wrote:
>>
>>> I'd like to request a review of 8131062 for jdk8u.  This patch can
>>> accelerate AES/GCM 4~5 times on armv8 by leveraging NEON isntructions.
>>> It can't apply to jdkj8u/hotspot cleanly, but it's trivial to integrate
>>> it. I just adjust code locations.
>>
>> I'm thinking of approving this, even though 8u is in long-term maintenance
>> mode so such enhancements wouldn't normally be appropriate.
>>
>> Here's my thinking: not only is AES horribly slow in software, it is also
>> vulnerable to timing attacks. In addition, encryption is a significant
>> consumer of time and power, and bottlenecks some applications. For that
>> reason, this patch should go in now, even though it wasn't back ported at
>> the time because 8u was already a maintenance release.
>>
>> However, this is an old version of the GHASH intrinsic. You really want
>> 8134869: AARCH64: GHASH intrinsic is not optimal.
>> https://bugs.openjdk.java.net/browse/JDK-8134869
>>
>> It'll need a jdk8u-fix-request tag.
>>
>> --
>> Andrew Haley  (he/him)
>> Java Platform Lead Engineer
>> Red Hat UK Ltd. <https://www.redhat.com>
>> https://keybase.io/andrewhaley
>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>>
>