[15] RFR(M): 8245512: CRC32 optimization using AVX512 instructions

[Vladimir Kozlov]

Testing passed. You can push it.

Regards,
Vladimir

On 6/4/20 10:16 AM, Vladimir Kozlov wrote:
> Looks good. I submitted our testing. I will let you know when it finished.
> 
> Regards,
> Vladimir
> 
> On 6/4/20 9:19 AM, Rukmannagari, Shravya wrote:
>> Hi Vladimir,
>> Please find the updated webrev below which now checks for sse4_1 support.
>> http://cr.openjdk.java.net/~srukmannagar/CRC32/webrev.03/
>>
>> Thanks,
>> Shravya.
>>
>> -----Original Message-----
>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>> Sent: Wednesday, June 3, 2020 3:28 PM
>> To: Rukmannagari, Shravya <shravya.rukmannagari at intel.com>; 'hotspot compiler' <hotspot-compiler-dev at openjdk.java.net>
>> Cc: Tucker, Greg B <greg.b.tucker at intel.com>
>> Subject: Re: [15] RFR(M): 8245512: CRC32 optimization using AVX512 instructions
>>
>> Then you have to check for VM_Version::supports_sse4_1() instead of UseSSE flag.
>>
>> Note, setting UseSSE flag lower disable 4.1 and 4.2 [1] the same way setting lower UseAVX disable AVX features [2].
>>
>> Thanks,
>> Vladimir
>>
>> [1] http://hg.openjdk.java.net/jdk/jdk/file/839d49bd8d8d/src/hotspot/cpu/x86/vm_version_x86.cpp#l644
>> [2] http://hg.openjdk.java.net/jdk/jdk/file/839d49bd8d8d/src/hotspot/cpu/x86/vm_version_x86.cpp#l695
>>
>> On 6/3/20 3:03 PM, Rukmannagari, Shravya wrote:
>>> Hi Vladimir,
>>> I have verified that the code does not use SSE4.2, whereas it uses SSE4.1 instruction set.
>>>
>>> Thanks,
>>> Shravya.
>>>
>>> -----Original Message-----
>>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>>> Sent: Tuesday, June 2, 2020 10:09 PM
>>> To: Rukmannagari, Shravya <shravya.rukmannagari at intel.com>; 'hotspot
>>> compiler' <hotspot-compiler-dev at openjdk.java.net>
>>> Cc: Tucker, Greg B <greg.b.tucker at intel.com>
>>> Subject: Re: [15] RFR(M): 8245512: CRC32 optimization using AVX512
>>> instructions
>>>
>>> How did it fail? UseSSE setting does not affect AVX settings. It seems you are using instructions from sse4.2 but not 
>>> checking for that.
>>>
>>> Vladimir
>>>
>>> On 6/2/20 6:15 PM, Rukmannagari, Shravya wrote:
>>>> Hi Vladimir,
>>>> The compiler/cpuflags/TestSSE4Disabled.java jtreg test was failing without the check.
>>>> This test is run with SSE=3 as:
>>>> run main/othervm -Xcomp -XX:UseSSE=3
>>>> compiler.cpuflags.TestSSE4Disabled
>>>> Without the UseSSE > 3 check, the JVM tries to generate the new AVX512 CRC32 stub.
>>>>
>>>> Thanks,
>>>> Shravya.
>>>>
>>>> -----Original Message-----
>>>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>>>> Sent: Tuesday, June 2, 2020 5:00 PM
>>>> To: Rukmannagari, Shravya <shravya.rukmannagari at intel.com>; 'hotspot
>>>> compiler' <hotspot-compiler-dev at openjdk.java.net>
>>>> Cc: Tucker, Greg B <greg.b.tucker at intel.com>
>>>> Subject: Re: [15] RFR(M): 8245512: CRC32 optimization using AVX512
>>>> instructions
>>>>
>>>> On 6/2/20 3:56 PM, Rukmannagari, Shravya wrote:
>>>>> Hi Vladimir,
>>>>> Thanks a lot for the review. I have modified the patch as per your comments. The CRC32 code is now in 
>>>>> macroAssembler_x86.cpp.
>>>>> http://cr.openjdk.java.net/~srukmannagar/CRC32/webrev.02/
>>>>
>>>> Why you added UseSSE check in stubGenerator_x86_64.cpp?
>>>>
>>>> +    if (UseSSE > 3 && VM_Version::supports_avx512_vpclmulqdq() &&
>>>>
>>>>>
>>>>> The stubGenerator_x86_64.cpp would be verified only for 64-bit builds. I have verified the 32-bit builds and also 
>>>>> ran the test cases to ensure no issues or failures.
>>>>
>>>> You are right about this.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>> Please let me know if you have questions or comments.
>>>>>
>>>>> Thanks,
>>>>> Shravya.
>>>>>
>>>>> -----Original Message-----
>>>>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>>>>> Sent: Monday, June 1, 2020 2:36 PM
>>>>> To: Rukmannagari, Shravya <shravya.rukmannagari at intel.com>; 'hotspot
>>>>> compiler' <hotspot-compiler-dev at openjdk.java.net>
>>>>> Cc: Tucker, Greg B <greg.b.tucker at intel.com>
>>>>> Subject: Re: [15] RFR(M): 8245512: CRC32 optimization using AVX512
>>>>> instructions
>>>>>
>>>>> Hi Shravya,
>>>>>
>>>>> Why you put new CRC32 avx512 code into macroAssembler_x86_aes.cpp file?
>>>>> This file is used only for AES intrinsic code - nothing else should be there.
>>>>>
>>>>> If you think CRC32 code is too large for macroAssembler_x86.cpp I would suggest to move all CRC32 code, old and 
>>>>> new, into new macroAssembler_x86_crc32.cpp file.
>>>>>
>>>>> I see that you want to implement new code only for 64 bit which is fine and you guarded it correctly wiht #ifrdef 
>>>>> _LP64.
>>>>> But you forgot guard in stubGenerator_x86_64.cpp which will cause build failure for 32-bit.
>>>>>
>>>>> It is difficult to judge the implementation code. I hope you ran all tests for it.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 5/20/20 4:01 PM, Rukmannagari, Shravya wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> We would like to contribute optimizations for CRC32 algorithm for upcoming Intel x86_64 platforms.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Contributors:
>>>>>>
>>>>>> Shravya Rukmannagari(shravya.rukmannagari at intel.com)
>>>>>>
>>>>>> Greg B Tucker(greg.b.tucker at intel.com)
>>>>>>
>>>>>>
>>>>>>
>>>>>> I have tested the patch to confirm correctness and performance. The patch also passes compiler/jtreg tests.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Please take a look and let me know if you have any questions or comments.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Bug Id: https://bugs.openjdk.java.net/browse/JDK-8245512
>>>>>>
>>>>>> https://cr.openjdk.java.net/~srukmannagar/CRC32/webrev.01/
>>>>>>
>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Shravya Rukmannagari
>>>>>>