[jdk17] RFR: 8269568: JVM crashes when running VectorMask query tests

[Jie Fu]

On Thu, 1 Jul 2021 00:25:27 GMT, Jie Fu <jiefu at openjdk.org> wrote:

>>> @XiaohongGong The following should fix this for x86:
>>> 
>>> ```
>>> diff --git a/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp b/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp
>>> index 147bcb8..b548877 100644
>>> --- a/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp
>>> +++ b/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp
>>> @@ -3870,6 +3870,9 @@ void C2_MacroAssembler::vector_mask_operation(int opc, Register dst, XMMRegister
>>>    vpsubb(xtmp, xtmp, mask, vec_enc);
>>>    evpmovb2m(ktmp, xtmp, vec_enc);
>>>    kmovql(tmp, ktmp);
>>> +  if (masklen < 64) {
>>> +    andq(tmp, (((jlong)1 << masklen) - 1));
>>> +  }
>>>    switch(opc) {
>>>      case Op_VectorMaskTrueCount:
>>>        popcntq(dst, tmp);
>>> @@ -3894,6 +3897,9 @@ void C2_MacroAssembler::vector_mask_operation(int opc, Register dst, XMMRegister
>>>    vpxor(xtmp, xtmp, xtmp, vec_enc);
>>>    vpsubb(xtmp, xtmp, mask, vec_enc);
>>>    vpmovmskb(tmp, xtmp, vec_enc);
>>> +  if (masklen < 64) {
>>> +    andq(tmp, (((jlong)1 << masklen) - 1));
>>> +  }
>>>    switch(opc) {
>>>      case Op_VectorMaskTrueCount:
>>>        popcntq(dst, tmp);
>>> ```
>>> 
>>> Please include this in your PR.
>> 
>> The fix seems to be tricky.
>> So do you think we need to change the code-gen like this https://github.com/openjdk/jdk17/pull/168#issuecomment-871412973 ?  @sviswa7
>
>> @DamonFool This is the cleanest solution I could think of.
>> The problem comes only with newly added intrinsic where firstTrue, lastTrue, trueCount become consumers to VectorStoreMask. All the other consumers of VectorStoreMask consume the right number of bits.
>> The fix is to make the new intrinsic also consume the right number of bits by doing the 'and' masking.
> 
> But we actually do not `and` masking with AVX3.
> So I'm afraid the root cause is the incorrect code-gen for VectorStoreMask.

> @DamonFool This small patch fixes the problem at hand and at the minimum should go in with JDK 17. You have filed the JDK-8269679 for a more robust fix. Also the masking support is being overhauled as part of JDK 18. Your thoughts?

Actually, instruct like `storeMask1B` and `storeMask2B` don't need `and masking` for both AVX2 and AVX3.
But to lower the risk of JDK17, I agree with you.
Thanks.

-------------

PR: https://git.openjdk.java.net/jdk17/pull/168