RFR: 8248188: Add IntrinsicCandidate and API for Base64 decoding [v3]

[Martin Doerr]

On Thu, 8 Oct 2020 06:53:26 GMT, CoreyAshford <github.com+51754783+CoreyAshford at openjdk.org> wrote:

>> This patch set encompasses the following commits:
>> 
>> - Adds a new HotSpot intrinsic candidate to the java.lang.Base64 class - decodeBlock(), and provides a flexible API for
>>   the intrinsic.  The API is similar to the existing encodeBlock intrinsic.
>> - Adds the code in HotSpot to check and martial the new intrinsic's arguments to the arch-specific intrinsic
>>   implementation
>> - Adds a Power64LE-specific implementation of the decodeBlock intrinsic.
>> - Adds a JMH microbenchmark for both Base64 encoding and encoding.
>> - Enhances the JTReg hotspot intrinsic "TestBase64.java" regression test to more fully test both decoding and encoding.
>
> CoreyAshford has updated the pull request incrementally with seven additional commits since the last revision:
> 
>  - stubGenerator_ppc.cpp: Fix multiple issues as per Martin Doerr's v2 review
>    
>     * Remove extraneous comma from SAP copyright notice
>     * Move align(32) to the head of the loop rather than the beginning of the unwound code
>     * Simplified looping condition to use a loop counter instead of a final
>       address.  This eliminated the need for the "end" variable, and
>       essentially replaced it with CTR, which is computed using a simple
>       bitwise shift of the size.
>     * Re-ran benchmarks against loop_unrolls values: 1, 2, 4, 8, 16 to find
>       optimal value, now 4.
>     * Corrected a typo in the word "elements"
>  - vm_version_ppc.cpp: per Martin Doerr's review of v2: fix copy/paste error
>  - vmIntrinsics.cpp: Per Martin Doerr's v2 review: rearrange order of case statement to be consistent with others.
>  - runtime.cpp: per Martin Doerr's review of v2, correct comment as per current semantics of decodeBlock()
>    
>      * The reference to "ofs" seems to be a copy/paste error.
>      * -1 is no longer returned from decodeBlock() in the event of a
>        non-base64 character being encountered; only a count of bytes written
>        to dst.
>  - TestBase64.java: Change comment as per Martin Doerr's v2 review
>  - Base64.java: Make changes as per Roger Riggs and Martin Doerr's v2 Review
>    
>      * Make comment about the sl parameter more precise
>      * Fix comparison to avoid possible integer overflow of sp
>  - library_call.cpp: Fix rebase merge error

Changes requested by mdoerr (Reviewer).

src/hotspot/cpu/ppc/stubGenerator_ppc.cpp line 3806:

> 3804:     // Load CTR with the number of passes through the unrolled loop
> 3805:     // = sl >> block_size_shift
> 3806:     __ srawi(sl, sl, block_size_shift);

Thanks, this is more simple. Unfortunately, sl can become 0. So I think we should move these 2 lines down before the
align and use: srawi_
beq(CCR0, unrolled_loop_exit)
mtctr

test/hotspot/jtreg/compiler/intrinsics/base64/TestBase64.java line 90:

> 88:
> 89:         // This should be enough to get both encodeBlock() and
> 90:         // decodeBlock() compiled on the highest tier.

It's actually encode() and decode() which should get compiled. You should see them when testing
with -XX:+PrintCompilation. And you should see usage of the intrinsics by -XX:+PrintInlining.

-------------

PR: https://git.openjdk.java.net/jdk/pull/293