RFR (XS): 8215888: Register to register spill may use AVX 512 move instruction on unsupported platform

[Viswanathan, Sandhya]

Thanks a lot!  I have also created backport patches for JDK 12 and JDK 11.0.2 as this bug affects those versions too. The below are for your consideration:

JDK 12:
http://cr.openjdk.java.net/~sviswanathan/8215888/jdk12/webrev.01/
JDK11u:
http://cr.openjdk.java.net/~sviswanathan/8215888/jdk11u/webrev.01/

The compiler jtreg testing passes for these as well. 

Best Regards,
Sandhya


-----Original Message-----
From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com] 
Sent: Friday, December 21, 2018 4:27 PM
To: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; hotspot compiler <hotspot-compiler-dev at openjdk.java.net>; vladimir.kozlov at oracle.com
Subject: Re: RFR (XS): 8215888: Register to register spill may use AVX 512 move instruction on unsupported platform


> Please find the updated webrev with your comments incorporated at:
> 
> http://cr.openjdk.java.net/~sviswanathan/8215888/webrev.01/

Thanks, submitted for testing.

Best regards,
Vladimir Ivanov

> -----Original Message-----
> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
> Sent: Friday, December 21, 2018 12:00 PM
> To: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; hotspot compiler <hotspot-compiler-dev at openjdk.java.net>; vladimir.kozlov at oracle.com
> Subject: Re: RFR (XS): 8215888: Register to register spill may use AVX 512 move instruction on unsupported platform
> 
> Sandhya,
> 
> I'd prefer to see the check inverted:
> 
>       if (UseAVX > 2 && !VM_Version::supports_avx512vl()) {
>         int vector_len = 2;
>         __ evmovdquq($dst$$XMMRegister, $src$$XMMRegister, vector_len);
>       } else {
>         __ movdqu($dst$$XMMRegister, $src$$XMMRegister);
>       }
> 
> It looks easier to read considering the code around is full of "UseAVX > 2" checks.
> 
> By coincidence I was debugging the very same bug today and at first I didn't notice the problem with "UseAVX < 2" misreading it as "UseAVX > 2".
> 
> Otherwise, looks good.
> 
> Best regards,
> Vladimir Ivanov
> 
> On 21/12/2018 11:44, Viswanathan, Sandhya wrote:
>> Hi All,
>>
>> We noticed that the register to register moves in x86.ad file attempt
>> to generate emovdqu when UseAVX==2.
>>
>> The instruction emovdquq is only supported on platforms where UseAVX >
>> 2 (AVX 512).
>>
>> The following rules in x86.ad file need to be corrected:
>>
>> MoveVecX2Leg
>>
>> MoveLeg2VecX
>>
>> MoveVecY2Leg
>>
>> MoveLeg2VecY
>>
>> The above move rules when activated through register allocator could
>> result in illegal instruction exception.
>>
>> Bug:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8215888
>>
>> This bug affects versions 11.0.2, 12 and the mainline.
>>
>> Webrev for jdk mainline:
>>
>> http://cr.openjdk.java.net/~sviswanathan/8215888/webrev.00/
>>
>> This webrev passes jtreg compiler tests on Haswell and SKX.
>>
>> Best Regards,
>>
>> Sandhya
>>