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.koz...@oracle.com] Sent: Monday, June 25, 2018 10:48 AM To: Kamath, Smita <smita.kam...@intel.com> Cc: hotspot compiler <hotspot-compiler-...@openjdk.java.net>; core-libs-dev@openjdk.java.net; Paul Sandoz <paul.san...@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.koz...@oracle.com] > Sent: Friday, June 22, 2018 12:29 PM > To: Kamath, Smita <smita.kam...@intel.com> > Cc: hotspot compiler <hotspot-compiler-...@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 >>
