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

[Vladimir Kozlov]

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
>      >>
>