JDK-8259610 : VectorReshapeTests are not effective due to failing to intrinsify "VectorSupport.convert"
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 17,18
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2021-01-12
  • Updated: 2021-12-16
  • Resolved: 2021-12-13
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 19
19 b02Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Description
Inlining "VectorSupport.convert()" needs all the arguments are constants, see the check at the begining:
   
 bool LibraryCallKit::inline_vector_convert() {
      const TypeInt*     opr                            = gvn().type(argument(0))->is_int();

      const TypeInstPtr* vector_klass_from = gvn().type(argument(1))->is_instptr();
      const TypeInstPtr* elem_klass_from   = gvn().type(argument(2))->is_instptr();
      const TypeInt*     vlen_from                 = gvn().type(argument(3))->is_int();

      const TypeInstPtr* vector_klass_to     = gvn().type(argument(4))->is_instptr();
      const TypeInstPtr* elem_klass_to       = gvn().type(argument(5))->is_instptr();
      const TypeInt*     vlen_to                     = gvn().type(argument(6))->is_int();

      if (!opr->is_con() ||
          vector_klass_from->const_oop() == NULL || elem_klass_from->const_oop() == NULL || !vlen_from->is_con() ||
          vector_klass_to->const_oop() == NULL || elem_klass_to->const_oop() == NULL || !vlen_to->is_con()) {
          if (C->print_intrinsics()) {
              tty->print_cr("  ** missing constant: opr=%s vclass_from=%s etype_from=%s vlen_from=%s vclass_to=%s etype_to=%s vlen_to=%s",
                                  NodeClassNames[argument(0)->Opcode()],
                                  NodeClassNames[argument(1)->Opcode()],
                                  NodeClassNames[argument(2)->Opcode()],
                                  NodeClassNames[argument(3)->Opcode()],
                                  NodeClassNames[argument(4)->Opcode()],
                                  NodeClassNames[argument(5)->Opcode()],
                                  NodeClassNames[argument(6)->Opcode()]);
         }
         return false; // not enough info for intrinsification
      }
    ...
}

These arguments all come from the "VectorSpecies" at the begining of the VectorAPI. To make sure the api can be intrinsifed, the VectorSpecies is needed to be constant as well.

However, most of the tests in "test/jdk/jdk/incubator/vector/VectorReshapeTests.java" uses the VectorSpecies as the method argument. And they will be used to get the vector type information for "VectorSupport.convert" finally. For example:

    @ForceInline
    static <E>
    void testVectorReshape(VectorSpecies<E> a, VectorSpecies<E> b, byte[] input, byte[] output) {
        testVectorReshape(a, b, input, output, false);
        testVectorReshapeLanewise(a, b, input, output);
    }
    ...
    @Test(dataProvider = "byteUnaryOpProvider")
    static void testReshapeByte(IntFunction<byte[]> fa) {
        byte[] bin64 = fa.apply(64/Byte.SIZE);
        byte[] bin128 = fa.apply(128/Byte.SIZE);
        byte[] bin256 = fa.apply(256/Byte.SIZE);
        byte[] bin512 = fa.apply(512/Byte.SIZE);
        byte[] binMax = fa.apply(S_Max_BIT.vectorBitSize()/Byte.SIZE);
        byte[] bout64 = new byte[bin64.length];
        byte[] bout128 = new byte[bin128.length];
        byte[] bout256 = new byte[bin256.length];
        byte[] bout512 = new byte[bin512.length];
        byte[] boutMax = new byte[binMax.length];

        for (int i = 0; i < NUM_ITER; i++) {
            testVectorReshape(bspec64, bspec64, bin64, bout64);
            testVectorReshape(bspec64, bspec128, bin64, bout128);
            testVectorReshape(bspec64, bspec256, bin64, bout256);
            testVectorReshape(bspec64, bspec512, bin64, bout512);
            testVectorReshape(bspec64, bspecMax, bin64, boutMax);

            ...
        }
    }

Even the methods is annotated with "ForceInline", the inline might fail I think. If so, the compiler might not treat the VectorSpecies as a constant, and the "VectorSupport.convert" will fail to be intrinsified and go back to the java implementation. This makes the whole tests non-effective. 

I'v tested it by adding some error in the ad file for cast/reinterpret match rules, and the issues are not reported as expected when running the VectorReshapeTests.java.
Comments
Changeset: ca8c58c7 Author: merykitty <anhmdq99@gmail.com> Committer: Paul Sandoz <psandoz@openjdk.org> Date: 2021-12-13 16:34:37 +0000 URL: https://git.openjdk.java.net/jdk/commit/ca8c58c731959e3a1b8fe02255ed44fc1d14d565
13-12-2021

[~psandoz], I'm sorry that I didn't look at the Vector*ConversionTests. I observed this issue when I was trying to add a jtreg test for https://bugs.openjdk.java.net/browse/JDK-8259353. This is the prototype tests: ``` @ForceInline static <E> void testReinterpretReshapeTwice(VectorSpecies<E> a, VectorSpecies<E> b, byte[] input, byte[] output) { Vector<E> av = a.fromByteArray(input, 0, ByteOrder.nativeOrder()); Vector<E> bv = av.reinterpretShape(b, 0); Vector<E> cv = bv.reinterpretShape(a, 0); cv.intoByteArray(output, 0, ByteOrder.nativeOrder()); int aSize = a.vectorByteSize(); int bSize = b.vectorByteSize(); int resize = Math.min(aSize, bSize); byte[] expected = new byte[aSize]; System.arraycopy(input, 0, expected, 0, resize); System.out.println("input: "+Arrays.toString(input)); System.out.println("expect: "+Arrays.toString(expected)); System.out.println("output: "+Arrays.toString(output)); Assert.assertEquals(output, expected); } @Test(dataProvider = "byteUnaryOpProvider") static void testReinterpretShapeTwice(IntFunction<byte[]> fa) { byte[] bin64 = fa.apply(64/Byte.SIZE); byte[] bin128 = fa.apply(128/Byte.SIZE); byte[] bin256 = fa.apply(256/Byte.SIZE); byte[] bin512 = fa.apply(512/Byte.SIZE); byte[] binMax = fa.apply(S_Max_BIT.vectorBitSize()/Byte.SIZE); byte[] bout64 = new byte[bin64.length]; byte[] bout128 = new byte[bin128.length]; byte[] bout256 = new byte[bin256.length]; byte[] bout512 = new byte[bin512.length]; byte[] boutMax = new byte[binMax.length]; for (int i = 0; i < NUM_ITER; i++) { testReinterpretReshapeTwice(bspec128, bspec64, bin128, bout128); testReinterpretReshapeTwice(bspec128, bspec256, bin128, bout128); testReinterpretReshapeTwice(bspec128, bspec512, bin128, bout128); testReinterpretReshapeTwice(bspec128, bspecMax, bin128, bout128); testReinterpretReshapeTwice(bspec256, bspec64, bin256, bout256); testReinterpretReshapeTwice(bspec256, bspec128, bin256, bout256); testReinterpretReshapeTwice(bspec256, bspec512, bin256, bout256); testReinterpretReshapeTwice(bspec256, bspecMax, bin256, bout256); testReinterpretReshapeTwice(bspec512, bspec64, bin512, bout512); testReinterpretReshapeTwice(bspec512, bspec128, bin512, bout512); testReinterpretReshapeTwice(bspec512, bspec256, bin512, bout512); testReinterpretReshapeTwice(bspec512, bspecMax, bin512, bout512); testReinterpretReshapeTwice(bspecMax, bspec64, binMax, boutMax); testReinterpretReshapeTwice(bspecMax, bspec128, binMax, boutMax); testReinterpretReshapeTwice(bspecMax, bspec256, binMax, boutMax); testReinterpretReshapeTwice(bspecMax, bspec512, binMax, boutMax); } } ``` I found that the issue cannot be reported as expected without any fixes. With "-XX:+PrintIntrinsics", I found the "convert" is not intrinsified sucessfully which uses the java implementation. And I also tried to delete part implementations for vector cast/reinterpret on ARM NEON, and the issues (should be bad ad file) are also not reported as expected.
14-01-2021

[~xgong] do you observe the same for the Vector*ConversionTests?
13-01-2021

[~psandoz], [~epavlova], [~sviswanathan] could someone please take care of this?
13-01-2021

ILW = Test fails to trigger C2 intrinsic (missed test coverage), vector API tests in VectorReshapeTests, no workaround = MLH = P4
13-01-2021