On Thu, 22 Oct 2020 22:06:11 GMT, CoreyAshford 
<github.com+51754783+coreyashf...@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

Reply via email to