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

[Vladimir Kozlov]

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
>>>