RFR: 8265154: vinserti128 operand mix up for KNL platforms

Sandhya Viswanathan sviswanathan at openjdk.java.net
Fri Apr 16 18:52:39 UTC 2021


On Wed, 14 Apr 2021 15:23:26 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> There is a bug in macro assembler in vinserti128 special handling for platforms like KNL that do not support AVX512VL.
>>  
>> The following:
>>    void vinserti128(XMMRegister dst, XMMRegister nds, XMMRegister src, uint8_t imm8) {
>>      if (UseAVX > 2 && VM_Version::supports_avx512novl()) {
>>        Assembler::vinserti32x4(dst, dst, src, imm8);
>>      }
>>      ...
>>   }
>>  
>> Should have been:
>>    void vinserti128(XMMRegister dst, XMMRegister nds, XMMRegister src, uint8_t imm8) {
>>      if (UseAVX > 2 && VM_Version::supports_avx512novl()) {
>>        Assembler::vinserti32x4(dst, nds, src, imm8);
>>      }
>>     ...
>>   }
>> 
>> Best Regards,
>> Sandhya
>
> What problems this is causing? Would be nice to have a test to show the issue (if possible).

@vnkozlov  @TobiHartmann Thanks a lot for the review.
I noticed this bug while running the vector api tests on KNL platform.
The specific tests that are failing without this fix are:
 jdk/incubator/vector/Float256VectorTests.java (withFloat256VectorTests)
 jdk/incubator/vector/Double256VectorTests.java (withDouble256VectorTests)

Majority of the places where vinserti128 is used in code gen, the dst and nds are passed as same register. Only in the implementation of VectorInsert node for vector api, the dst and nds are different.

The vector api tests are run as part of tier 3.

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

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


More information about the hotspot-compiler-dev mailing list