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

[Viswanathan, Sandhya]

Thanks a lot Tobias! I will work with Vivek to push the patch to jdk/jdk12.

Best Regards,
Sandhya

-----Original Message-----
From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com] 
Sent: Thursday, January 03, 2019 1:01 AM
To: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; Vladimir Ivanov <vladimir.x.ivanov at oracle.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

Hi Sandhya,

all webrevs look good to me (and the tests submitted by Vladimir passed).

You don't need to push to JDK 13 because patches pushed to JDK 12 will be synced with mainline
automatically:
https://mail.openjdk.java.net/pipermail/jdk-dev/2018-December/002376.html

So I would suggest to push the patch to jdk/jdk12 and request a backport to JDK 11u after some iterations of nightly testing have passed.

Best regards,
Tobias


On 22.12.18 01:55, Viswanathan, Sandhya wrote:
> 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
>>>