On Fri, 18 Oct 2024 02:03:21 GMT, Jatin Bhateja <jbhat...@openjdk.org> wrote:

>> This patch optimizes LongVector multiplication by inferring VPMULUDQ 
>> instruction for following IR pallets.
>>   
>> 
>>        MulL   ( And  SRC1,  0xFFFFFFFF)   ( And  SRC2,  0xFFFFFFFF) 
>>        MulL   (URShift SRC1 , 32) (URShift SRC2, 32)
>>        MulL   (URShift SRC1 , 32)  ( And  SRC2,  0xFFFFFFFF)
>>        MulL   ( And  SRC1,  0xFFFFFFFF) (URShift SRC2 , 32)
>> 
>> 
>> 
>>  A  64x64 bit multiplication produces 128 bit result, and can be performed 
>> by individually multiplying upper and lower double word of multiplier with 
>> multiplicand and assembling the partial products to compute full width 
>> result. Targets supporting vector quadword multiplication have separate 
>> instructions to compute upper and lower quadwords for 128 bit result. 
>> Therefore existing VectorAPI multiplication operator expects shape 
>> conformance between source and result vectors.
>> 
>> If upper 32 bits of quadword multiplier and multiplicand is always set to 
>> zero then result of multiplication is only dependent on the partial product 
>> of their lower double words and can be performed using unsigned 32 bit 
>> multiplication instruction with quadword saturation. Patch matches this 
>> pattern in a target dependent manner without introducing new IR node.
>>  
>> VPMULUDQ instruction performs unsigned multiplication between even numbered 
>> doubleword lanes of two long vectors and produces 64 bit result.  It has 
>> much lower latency compared to full 64 bit multiplication instruction 
>> "VPMULLQ", in addition non-AVX512DQ targets does not support direct quadword 
>> multiplication, thus we can save redundant partial product for zeroed out 
>> upper 32 bits. This results into throughput improvements on both P and E 
>> core Xeons.
>> 
>> Please find below the performance of [XXH3 hashing benchmark 
>> ](https://mail.openjdk.org/pipermail/panama-dev/2024-July/020557.html)included
>>  with the patch:-
>>  
>> 
>> Sierra Forest :-
>> ============
>> Baseline:-
>> Benchmark                                 (SIZE)   Mode  Cnt    Score   
>> Error   Units
>> VectorXXH3HashingBenchmark.hashingKernel    1024  thrpt    2  806.228        
>>   ops/ms
>> VectorXXH3HashingBenchmark.hashingKernel    2048  thrpt    2  403.044        
>>   ops/ms
>> VectorXXH3HashingBenchmark.hashingKernel    4096  thrpt    2  200.641        
>>   ops/ms
>> VectorXXH3HashingBenchmark.hashingKernel    8192  thrpt    2  100.664        
>>   ops/ms
>> 
>> With Optimization:-
>> Benchmark                                 (SIZE)   Mode  Cnt     Score   
>> Error   Units
>> VectorXXH3HashingBenchmark.hashingKernel   ...
>
> Jatin Bhateja has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains two commits:
> 
>  - Review resolutions
>  - 8341137: Optimize long vector multiplication using x86 VPMULUDQ instruction

src/hotspot/share/opto/vectornode.cpp line 2122:

> 2120:     // MulL (URShift SRC1 , 32) (URShift SRC2, 32)
> 2121:     // MulL (URShift SRC1 , 32)  ( And  SRC2,  0xFFFFFFFF)
> 2122:     // MulL ( And  SRC1,  0xFFFFFFFF) (URShift SRC2 , 32)

I don't understand how it works... According to the documentation, 
`VPMULDQ`/`VPMULUDQ` consume vectors of double words and produce a vector of 
quadwords. But it looks like `SRC1`/`SRC2` are always vectors of longs 
(quadwords). And `vmuludq_reg` in `x86.ad` just takes the immedate operands and 
pass them into `vpmuludq` which doesn't look right...

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21244#discussion_r1805886268

Reply via email to