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

Paul Murphy github.com+12972156+pmur at openjdk.java.net
Mon Oct 26 19:47:21 UTC 2020


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

>> src/hotspot/cpu/ppc/stubGenerator_ppc.cpp line 3878:
>> 
>>> 3876:             // |    Element    |             |                      |                      |             |             |                      |                      |             |
>>> 3877:             // +===============+=============+======================+======================+=============+=============+======================+======================+=============+
>>> 3878:             // | after vaddubm | 00||b0:0..5 | 00||b0:6..7||b1:0..3 | 00||b1:4..7||b2:0..1 | 00||b2:2..7 | 00||b3:0..5 | 00||b3:6..7||b4:0..3 | 00||b4:4..7||b5:0..1 | 00||b5:2..7 |
>> 
>> An extra line here showing how the 8 6-bit values above get mapping into 6 bytes greatly help my brain out. (likewise for the <P10 case below too)
>
> Just to make sure I understand, you're not asking for a change here, is that right?

I think the first line should also express the initial layout of the 6 bit values similar to the linked algo.  I think changing this comment add an extra line which describes the bits as they leave `vaddubm` would be helpful to understand the demangling here. (e.g the `00aaaaaa 00bbbbbb 00ccccc 00dddddd` comments in the linked paper)

>> src/hotspot/cpu/ppc/stubGenerator_ppc.cpp line 3884:
>> 
>>> 3882:             // |   vec_0x3fs   |  00111111   |       00111111       |       00111111       |  00111111   |  00111111   |       00111111       |       00111111       |  00111111   |
>>> 3883:             // +---------------+-------------+----------------------+----------------------+-------------+-------------+----------------------+----------------------+-------------+
>>> 3884:             // | after vpextd  |   b5:0..7   |       b4:0..7        |       b3:0..7        |   b2:0..7   |   b1:0..7   |       b0:0..7        |       00000000       |  00000000   |
>> 
>> Are theses comments correct or am I misunderstanding this? I read the final result as something starting as `b5:2..7 || b4:4..7|| b5:0..1` from vpextd.
>
> Because the bytes are displayed e15..e8, instead of the other way around, it's hard to follow.  As an example, consider just the last four bytes of the table, but displayed in the reverse order:
> 
> 00||b0:0..5    00||b0:6..7||b1:0..3    00||b1:4..7||b2:0..1    00||b2:2..7
> 
> After vpextd with bit select pattern 00111111 for all bytes:
> 
> b0:0..5||b0:6..7    b1:0..3||1:4..7    b2:0..1||b2:2..7
> =
> b0:0..7    b1:0..7    b2:0..7
> 
> Should I reverse the order of this table with a comment at the top, to explain the reason for the reversal?  It seems like a good idea.

Since you are operating on doublewords here, expressing this as operations on a doubleword instead of bytes would be more intuitive here.  I think the lane mappings for little endian are what throw me off.

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

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


More information about the core-libs-dev mailing list