RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 Instructions

[Vladimir Kozlov]

Yes, this looks good to me. Let me test it.

Thanks,
Vladimir

On 6/25/18 10:25 PM, Kamath, Smita wrote:
> Hi Vladimir,
> 
> Please find the updated webrev link.
> 
> Webrev Link: http://cr.openjdk.java.net/~srukmannagar/Base64/webrev.00/
> Bug link: https://bugs.openjdk.java.net/browse/JDK-8205528
> 
> Please let me know if you have additional questions.
> 
> Thanks and Regards,
> Smita
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Monday, June 25, 2018 10:48 AM
> To: Kamath, Smita <smita.kamath at intel.com>
> Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>; core-libs-dev at openjdk.java.net; Paul Sandoz <paul.sandoz at oracle.com>
> Subject: Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 Instructions
> 
> I forgot to reply to your answers.
> 
> On 6/22/18 2:49 PM, Kamath, Smita wrote:
>> Hi Vladimir,
>>
>> Please see my answers to your questions as below:
>>
>> 1) One question so: why you have own copy of base64 charsets and not using one in library:
>> I am using vpgatherdd instruction to fetch multiple values from base64 array in a single instruction with a vector index. Vpgatherdd instruction works on 32 bit array and so I need to define base64 charset in a 32 bit array. I have given reference to gather instruction below.
> 
> As was discussed in an other e-mail lets keep your copy.
> 
>>
>> 2) Some indents in new and old Assembler::emit_operand() are off. In new Assembler::emit_operand() is better use } else { instead of 'return' in one branch.
>> I'll make the necessary changes and send an updated webrev.
>>
>> 3) This is first time I see that XMM register can be used for index in address. Is it true? Can you point to Intel's document which describes it.
>> I am using vpgatherdd instruction which requires index vector with scale. It uses VSIB addressing where the index register is a zmm register.
>> Please refer to reference manual, volume 2c, page 2211:
>> https://software.intel.com/sites/default/files/managed/39/c5/325462-sd
>> m-vol-1-2abcd-3abcd.pdf Also see, section 2.3.12, page 524 for VSIB
>> memory addressing information.
> 
> Got it. Thanks for document reference.
> 
>>
>> 4)  Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may be similar to sha or AES in compiler/codegen/aes) to make sure that all flags, intrinsic is used and it produces correct result.
>> I will add test cases as per your suggestion.
> 
> Thanks,
> Vladimir
> 
>>
>> Please let me know if you have additional questions.
>>
>> Thanks,
>> Smita
>>
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Friday, June 22, 2018 12:29 PM
>> To: Kamath, Smita <smita.kamath at intel.com>
>> Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
>> Subject: Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512
>> Instructions
>>
>> Hi Smita,
>>
>> I CCing to Libs to review code changes in Base64.java.
>>
>> Looks like you changed all need place to implement intrinsic.
>> One question so: why you have own copy of base64 charsets and not using one in library:
>>
>>             private int encode0(byte[] src, int off, int end, byte[] dst) {
>>                 char[] base64 = isURL ? toBase64URL : toBase64;
>>
>> Some indents in new and old Assembler::emit_operand() are off.
>>
>> In new Assembler::emit_operand() is better use } else { instead of 'return' in one branch.
>>
>> This is first time I see that XMM register can be used for index in address. Is it true? Can you point to Intel's document which describes it.
>>
>> What testing you did?
>>
>> Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may be similar to sha or AES in compiler/codegen/aes) to make sure that all flags, intrinsic is used and it produces correct result.
>>
>> I know there is test/jdk/java/util/Base64/ tests but they may not trigger intrinsic usage. But you can use them as starting point for new tests.
>>
>> Thanks,
>> Vladimir
>>
>> On 6/22/18 11:47 AM, Kamath, Smita wrote:
>>> Hi Vladimir,
>>>
>>> I'd like to contribute an optimization for Base64 Encoding Algorithm
>>> using AVX512 Instructions. This optimization shows 1.5x improvement
>>> on
>>> x86_64 platform(SKL).
>>>
>>> Link to Bug: https://bugs.openjdk.java.net/browse/JDK-8205528
>>>
>>> Link to webrev:
>>> http://cr.openjdk.java.net/~vdeshpande/Base64/webrev.00/
>>>
>>> For testing the implementation, I have run tests under
>>> test/jdk/java/util/Base64/ and also ran
>>> test/jdk/com/sun/jndi/ldap/Base64Test.java
>>>
>>> I have also run jtreg.
>>>
>>> Please review and let me know if you have any comments.
>>>
>>> Thanks and Regards,
>>>
>>> Smita
>>>