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

[Tobias Hartmann]

Thanks, Vladimir!

Best regards,
Tobias

On 01.06.2016 18:20, Vladimir Kozlov wrote:
> 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
>>>>