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

[Vladimir Kozlov]

On 6/22/18 3:58 PM, Paul Sandoz wrote:
Hi Smita,
I am ok with it if Vladimir is :-) One slight concern is this may be biasing 
towards the x86 implementation of the intrinsic.

Looking on code and it will be a lot of changes in Base64.java. I am 
concern about that late in JDK 11.

I think we should keep duplicated code for x86 intrinsic as Smita 
suggested. And we can return to this when/if we intrinsify Decoder too.

I dunno if an int[] table is as useful for an AARCH64 intrinsic.

We should ask RH to check.

But I think SPARC is better operating on 32-bit values than 16-bit (at 
least it was issue before).

Vladimir


Paul.

On Jun 22, 2018, at 3:25 PM, Kamath, Smita <smita.kam...@intel.com> wrote:

Hi Paul,

I can change the Java arrays(toBase64URL and toBase64) to int[] and use them in 
the intrinsic if it is acceptable. Please let me know.

Thanks,
Smita

-----Original Message-----
From: Paul Sandoz [mailto:paul.san...@oracle.com]
Sent: Friday, June 22, 2018 3:18 PM
To: Kamath, Smita <smita.kam...@intel.com>
Cc: Vladimir Kozlov <vladimir.koz...@oracle.com>; hotspot compiler 
<hotspot-compiler-...@openjdk.java.net>
Subject: Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 
Instructions



On Jun 22, 2018, at 3:00 PM, Kamath, Smita <smita.kam...@intel.com> wrote:
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;


Yes, especially if we converted those from char[] to byte[] (which might also 
improve the C2 generated code) and pass the selected byte[] to the intrinsic.
Smita>>  I need an integer array in order to use vpgatherdd instruction with 
vector index. Vpgather instruction works on 32 bit array and so I need to define 
base64 charset in a 32 bit array.


Thanks, I also saw your reply to Vladimir and see why you need this [*]. We 
could still unify leveraging a Java int[] array at the expense of extra space 
required on non-intrisified platforms. IMHO the less stub code and the more 
Java code the better with regards to maintenance.


Naming wise for the Java methods here are some suggestions:

generateImplEncode -> encodeBlockWithBoundsCheck  implEncode ->
encodeBlock

Also can generateImplEncode be private?
Smita>> I'll make these changes and send an updated webrev.


Ok.


Further. is there is a need to perform bounds checks in generateImplEncode 
given the public methods calling encode will, i presume, have dominating checks?
Smita>> The check is not required. I'll retain encodeBlock and remove 
encodeBlockWithBoundsCheck.


Ok.

Thanks,
Paul.

[*] On AVX-512 it's tempting to explore permute/rearrange operations on bytes, 
if there are any such instructions, since the translation array of bytes 
(toBase64URL or toBase64) fits neatly into one z register, or for AVX-2 in two 
y registers if some masked variant, based on ranges, is possible.