[9] RFR(S): 8158214: Crash with "assert(VM_Version::supports_sse4_1()) failed" if UseSSE < 4 is set

[Vladimir Kozlov]

Looks good.

Thanks,
Vladimir

On 6/1/16 7:03 AM, Tobias Hartmann wrote:
> Hi Vladimir,
>
> thanks for the review!
>
> On 31.05.2016 19:48, Vladimir Kozlov wrote:
>> Is it only pinsrd instruction requires sse4.1? I looked on crc32 code and we can use pinsrw instruction with shift:
>>
>> if (supports_sse4_1()) {
>>   pinsrd(xmm1, crc, 0);
>> } else {
>>   pinsrw(xmm1, crc, 0);
>>   shrl(crc, 16);
>>   pinsrw(xmm1, crc, 1);
>> }
>
> Right, 'pinsrd' is the only SSE 4.1 instruction in the crc32 code, so this works.
>
> Here's the new webrev:
> http://cr.openjdk.java.net/~thartmann/8158214/webrev.01/
>
> Thanks,
> Tobias
>
>> Unfortunately UseAESCTRIntrinsics use more sse4.1 instructions so we can't do that. And it is already guarded by supports_sse4_1().
>>
>> Changes which remove UseSSE checks in vm_version_x86.cpp are good.
>>
>> Thanks,
>> Vladimir
>>
>>
>> On 5/31/16 5:48 AM, Tobias Hartmann wrote:
>>> Hi,
>>>
>>> please review the following patch:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8158214
>>> http://cr.openjdk.java.net/~thartmann/8158214/webrev.00/
>>>
>>> The VM crashes during stub generation if UseSSE = 3 is set. The problem is that even if SSE 4 is not available or disabled, MacroAssembler::kernel_crc32() emits the 'pinsrd' instruction which is a SSE 4.1 instruction.
>>>
>>> The solution is to only emit CRC32 intrinsics if SSE 4.1 is available (the UseSSE > 2 check is not strong enough). I also removed the unnecessary UseSSE >= 4 checks because they are included in the supports_sse4_2() check.
>>>
>>> Tested with JPRT and RBT (running).
>>>
>>> Thanks,
>>> Tobias
>>>