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

[Sandhya Viswanathan]

On Wed, 30 Jun 2021 01:45:15 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> IIRC we wrote them as smoke tests because they were not intrinsic. We need to think more carefully about converting them from smoke tests.
>> 
>> Ideally we should convert them to kernel tests, but that is more work. Instead we can copy the generated pattern and do the following:
>> 
>> - move the assertion outside of the loops (it will generate garbage with string concatenation)
>> - assert over arrays, thereby also moving the actual scalar computation result outside the loops. The simplest approach is to create an `int[] array` of the same length as the input and write the reduced result at index `i`. Thus it's sparse.
>> 
>> That should result in an inner loop body that is very focused on exercising the intrinsic method. It will also likely reduce the test execution times.
>
>> IIRC we wrote them as smoke tests because they were not intrinsic. We need to think more carefully about converting them from smoke tests.
>> 
> Thanks for looking at this PR @PaulSandoz ! Yes, I think this need more work and more carefully to move them from the smoke tests. Maybe we can revisit them in future?
> 
>> Ideally we should convert them to kernel tests, but that is more work. Instead we can copy the generated pattern and do the following:
>> 
>> * move the assertion outside of the loops (it will generate garbage with string concatenation)
>> * assert over arrays, thereby also moving the actual scalar computation result outside the loops. The simplest approach is to create an `int[] array` of the same length as the input and write the reduced result at index `i`. Thus it's sparse.
>> 
>> That should result in an inner loop body that is very focused on exercising the intrinsic method. It will also likely reduce the test execution times.
> 
> Agree, I will move the assertion outside of the loop first. Thanks so much!

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

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

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