Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8262096: Vector API fails to work due to VectorShape initialization exception #2722

Closed
wants to merge 9 commits into from
14 changes: 11 additions & 3 deletions src/hotspot/share/prims/vectorSupport.cpp
Expand Up @@ -362,13 +362,21 @@ int VectorSupport::vop2ideal(jint id, BasicType bt) {
*/

JVM_ENTRY(jint, VectorSupport_GetMaxLaneCount(JNIEnv *env, jclass vsclazz, jobject clazz)) {
#ifdef COMPILER2
oop mirror = JNIHandles::resolve_non_null(clazz);
if (java_lang_Class::is_primitive(mirror)) {
BasicType bt = java_lang_Class::primitive_type(mirror);
return Matcher::max_vector_size(bt);
}
int min_lane_count = 64 / type2aelembytes(bt);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am uncertain of the units here. Is the numerator in bits and the denominator in bytes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Paul for your review.

Oops, the numerator should be 8 (bytes).

Updated.
Testing of jdk/incubator/vector (MaxVector=default/8/4 or without C2) is still fine.
Thanks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, was the test VectorShapeInitTest passing prior to the fix of the numerator?
Perhaps we should be testing more directly on VectorShape.S_Max_BIT.vectorBitSize() and VectorShape.preferredShape ?
Also, perhaps we can run with C2 disabled, with -XX:TieredStopAtLevel=3 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, was the test VectorShapeInitTest passing prior to the fix of the numerator?
Yes. It also passed with min_lane_count = 64 / type2aelembytes(bt).

Perhaps we should be testing more directly on VectorShape.S_Max_BIT.vectorBitSize() and VectorShape.preferredShape ?
Ok.
But it that case, I'd like to just re-use jdk/incubator/vector/PreferredSpeciesTest.java.

Also, perhaps we can run with C2 disabled, with -XX:TieredStopAtLevel=3 ?
Fine.
Although this bug wouldn't be triggered with -XX:TieredStopAtLevel=x, it can help test with C1.

Patch had been updated.
Any comments?
Thanks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reusing PreferredSpeciesTest is a good idea.

I now realize that C2 needs to be compiled out to trigger some other cases. In that case I think we can remove the execution with -XX:TieredStopAtLevel=x. The test will get executed with various HotSpot configurations by the test infrastructure, eventually...

Looks good.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case I think we can remove the execution with -XX:TieredStopAtLevel=x.

Fixed. Thanks.

// The return value should not be less than min_lane_count since the minimal vector size
// supported by the Vector API is 64-bit (see VectorShape.preferredShape() for more details).
// This is fine because the vector size is unlikely to be less than 64-bit on modern hardware.
// And for the worst case, where the hardware doesn't support 64-bit long, the non-vector
// implementation will be called, which is still fine.
#ifdef COMPILER2
return MAX2(Matcher::max_vector_size(bt), min_lane_count);
#else
return min_lane_count;
#endif // COMPILER2
}
return -1;
} JVM_END

Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -246,11 +246,6 @@ private static VectorShape computePreferredShape() {
Class<?> etype = type.elementType;
int maxLaneCount = VectorSupport.getMaxLaneCount(etype);
int maxSize = type.elementSize * maxLaneCount;
// FIXME: Consider removing, since unlikely to occur on modern hardware
if (maxSize < Double.SIZE) {
String msg = "shape unavailable for lane type: " + etype.getName();
throw new UnsupportedOperationException(msg);
}
prefBitSize = Math.min(prefBitSize, maxSize);
}
// If these assertions fail, we must reconsider our API portability assumptions.
Expand Down
48 changes: 48 additions & 0 deletions test/hotspot/jtreg/compiler/vectorapi/VectorShapeInitTest.java
@@ -0,0 +1,48 @@
/*
* Copyright (C) 2021 THL A29 Limited, a Tencent company. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package compiler.vectorapi;

import jdk.incubator.vector.DoubleVector;
import jdk.incubator.vector.VectorOperators;
import jdk.incubator.vector.VectorSpecies;

/*
* @test
* @bug 8262096
* @summary Test the initialization of vector shapes
* @modules jdk.incubator.vector
* @run main/othervm -XX:MaxVectorSize=8 compiler.vectorapi.VectorShapeInitTest
* @run main/othervm -XX:MaxVectorSize=4 compiler.vectorapi.VectorShapeInitTest
*/

public class VectorShapeInitTest {
static final VectorSpecies<Double> SPECIES = DoubleVector.SPECIES_64;
static double[] a = new double[64];
static double[] r = new double[64];

public static void main(String[] args) {
DoubleVector av = DoubleVector.fromArray(SPECIES, a, 0);
av.lanewise(VectorOperators.ABS).intoArray(r, 0);
}
}