[8u] RFR: 8143925 : enhancing CounterMode.crypt() for AESCrypt.implEncryptBlock() + 8146581 : Minor corrections to the patch submitted for earlier bug id - 8143925

Martin Balao mbalao at redhat.com
Wed Jan 22 20:31:26 UTC 2020


On 1/17/20 9:38 AM, Anton Kozlov wrote:
> 
>> It worth noticing that this patch affects AArch64 too (for which I have
>> a minor patch).
> 
> As well as other CPUs :) For example, we will make any required change for aarch32-port, but only after anything actually pushed to jdk8u.

Yes, agree.

> 
>>
>>  * src/cpu/x86/vm/assembler_x86.cpp
>>   * 1st hook does not apply cleanly because 8210764 [2], 8132207 [3] and
>> 8145688 [4] are not in 8u. Manually applied changes.
>>   * 4th hook does not apply cleanly because 8132207 [3] is not in 8u.
>> Manually applied changes.
>>   * Removed all instruction attributes
> 
> Attributes in original patch hold rex_w parameter that is accepted by 8u's simd_prefix.
> I think you should pass true it at least in pextrq and pinsrq.

Well spotted. Agree.

> 
>>  * src/cpu/x86/vm/vm_version_x86.cpp
>>   * 3rd hook does not apply cleanly because 8134553 [5] is not in 8u.
>> Manually applied changes. Re-ordered the hooks and added conditionals to
>> keep the semantics.
> 
> You created a separate if/else. Although this is IMHO cleaner than the original patch, I think it will create difficulties applying following JDK-8146581.
> 
> Similar minor comment: you fixed indentation issue in original patch [1]. The cosmetic difference in code doesn't affect any jdk8u user. So there is no reason to deviate from original code, making further backports more complicated.

Yes, I re-worked that part because it was a bit confusing and hard to
reason about.

What I had taken into account was the following:

1) In baseline, the whole block is under "supports_aes()" and "UseAES"
--> looks good in your patch

2) UseAESIntrinsics was previously set --> looks good in your patch

3) UseAESCTRIntrinsics was not already used/set in the file --> looks
good in your patch

I've also had a look at the JDK part (which was still not done in my
backport) and looks good to me.

In conclusion, I'm good to go with your 01 version. I'm not an official
reviewer, though.

Thanks,
Martin.-



More information about the jdk8u-dev mailing list