RFR(XS): JDK-8207746: C2: Lucene crashes on AVX512 instruction: Fix

[Vladimir Kozlov]

To remove any other concerns about affects of using add/sub in macro instructions I decided to replace all places where 
zmm register is saved/restored on stack:

http://cr.openjdk.java.net/~kvn/8207746/webrev.02/

It may be overkill but we are very very late in JDK 11 release and the only other solution I see is disable AVX512 by 
default at this time.

I will test this version and give report.

Regards,
Vladimir

On 8/14/18 6:55 PM, Vladimir Kozlov wrote:
> Thank you, David
> 
> pcmpeq[b|w]() is not affected since it set result in XMM dst register and not flags.
> 
> pcmpestri() is affected and needs to be fixed  - there is flag check after it.
> 
> pmovzxbw() and pmovmskb() has nothing with flags.
> 
> But we need to verify that in all places where these methods are called there is no assumption that flags are preserved 
> over this methods (for example to have 'cmp' before 'pmovzxbw' and 'jcc' after it).
> 
> I become more and more concern about quality of our AVX512 code :(
> 
> Thanks,
> Vladimir
> 
> On 8/14/18 6:39 PM, David Schlosnagle wrote:
>> // outside observer, feel free to tell me I'm off base here
>>
>> Looking at the pattern of code change in this fix, are the following places also affected by register clobbering on 
>> the AVX 512 code paths?
>>
>>    MacroAssembler::pcmpeqb(XMMRegister dst, XMMRegister src)
>>    MacroAssembler::pcmpeqw(XMMRegister dst, XMMRegister src)
>>    MacroAssembler::pcmpestri(XMMRegister dst, Address src, int imm8)
>>    MacroAssembler::pmovzxbw(XMMRegister dst, XMMRegister src)
>>    MacroAssembler::pmovzxbw(XMMRegister dst, Address src)
>>    MacroAssembler::pmovmskb(Register dst, XMMRegister src)
>>
>> Thanks,
>> Dave
>>
>> On Tue, Aug 14, 2018 at 5:35 PM Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
>>
>>     Good. I will restart testing.
>>
>>     Thanks,
>>     Vladimir
>>
>>     On 8/14/18 2:25 PM, Deshpande, Vivek R wrote:
>>      > Hi Vladimir
>>      >
>>      > Thanks Vladimir for reviewing it and suggesting for other places.
>>      > We looked at other places and we have an updated webrev which fixes similar problem at 2 more locations.
>>      > The updated webrev is here:
>>      > http://cr.openjdk.java.net/~vdeshpande/Lucene_avx512/webrev.01/
>>     <http://cr.openjdk.java.net/%7Evdeshpande/Lucene_avx512/webrev.01/>
>>      > I have also updated the bug.
>>      >
>>      > Regards,
>>      > Vivek
>>      >
>>      > -----Original Message-----
>>      > From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>]
>>      > Sent: Tuesday, August 14, 2018 11:18 AM
>>      > To: Deshpande, Vivek R <vivek.r.deshpande at intel.com <mailto:vivek.r.deshpande at intel.com>>;
>>     hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net> compiler
>>     <hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>>
>>      > Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com <mailto:sandhya.viswanathan at intel.com>>
>>      > Subject: Re: RFR(XS): JDK-8207746: C2: Lucene crashes on AVX512 instruction: Fix
>>      >
>>      > Thank you very much, Vivek, for your hard work to find the issue and fixing it.
>>      >
>>      > The fix totally makes sense. I will run all our test with it.
>>      >
>>      > Meanwhile can you check other places (intrinsics) where we operate on stack pointer to make sure that they don't
>>     have the same issue? It is not urgent and separate issue but I am now concern about how we use it in code.
>>      >
>>      > Thanks,
>>      > Vladimir
>>      >
>>      > On 8/14/18 10:37 AM, Deshpande, Vivek R wrote:
>>      >> Hi All
>>      >>
>>      >> I have a patch for the fix for Lucene crash using AVX512.
>>      >>
>>      >> You can find the webrev at this location:
>>      >>
>>      >> http://cr.openjdk.java.net/~vdeshpande/Lucene_avx512/webrev.00/
>>     <http://cr.openjdk.java.net/%7Evdeshpande/Lucene_avx512/webrev.00/>
>>      >>
>>      >> Could you please review the change.
>>      >>
>>      >> The EFLAGS register was getting clobbered, causing the result of
>>      >> vptest instruction getting clobbered, leading to crash.
>>      >>
>>      >> This fix avoids the clobbering of the result of the vptest instruction.
>>      >>
>>      >> I tested the fix with elastic-search docker container on skylake
>>      >> server. The container is running for more than 18 hours now.
>>      >>
>>      >> Regards,
>>      >>
>>      >> Vivek
>>      >>
>>