From 790fa68f76494f3c246b64ee5314105b5a0f1b1d Mon Sep 17 00:00:00 2001 From: Ningsheng Jian Date: Tue, 15 Mar 2022 15:30:19 +0800 Subject: [PATCH 1/2] 8282764: AArch64: compiler/vectorapi/reshape/TestVectorCastNeon.java failed with incorrect result Vector API long to float conversion operation on NEON converts vector of 2 longs to 2 floats. In current implementation, we convert 2 longs to 2 doubles first and then converts 2 doubles to 2 floats. However, this two-steps conversion may have two roundings, while the expected behavior, conversion from long to float directly, has only one rounding. This will result in inconsistent result. E.g. for the failure test case: ``` jshell> long l = 0x561a524000000001L; l ==> 6204361871487664129 jshell> float l2f = (float)l; l2f ==> 6.2043621E18 jshell> float l2d2f = (float)((double)l); l2d2f ==> 6.2043616E18 ``` Since we don't have NEON instruction to support vector long to float conversion, we fix the codegen by doing it in scalar operation one element by one element. TEST: vector api jtreg tests passed. --- src/hotspot/cpu/aarch64/aarch64_neon.ad | 17 ++++++++++++----- src/hotspot/cpu/aarch64/aarch64_neon_ad.m4 | 17 ++++++++++++----- .../vectorapi/VectorCastShape128Test.java | 3 ++- .../vectorapi/reshape/TestVectorCastAVX1.java | 3 ++- .../vectorapi/reshape/TestVectorCastAVX2.java | 3 ++- .../vectorapi/reshape/TestVectorCastAVX512.java | 3 ++- .../reshape/TestVectorCastAVX512BW.java | 3 ++- .../reshape/TestVectorCastAVX512DQ.java | 3 ++- .../vectorapi/reshape/TestVectorCastNeon.java | 5 +++-- .../vectorapi/reshape/TestVectorCastSVE.java | 3 ++- .../reshape/TestVectorReinterpret.java | 3 ++- .../reshape/utils/VectorReshapeHelper.java | 11 ++++++----- 12 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/hotspot/cpu/aarch64/aarch64_neon.ad b/src/hotspot/cpu/aarch64/aarch64_neon.ad index feecd8ab90add..c3995a220f31a 100644 --- a/src/hotspot/cpu/aarch64/aarch64_neon.ad +++ b/src/hotspot/cpu/aarch64/aarch64_neon.ad @@ -350,16 +350,23 @@ instruct vcvt4Bto4I(vecX dst, vecD src) ins_pipe(pipe_class_default); %} -instruct vcvt2Lto2F(vecD dst, vecX src) +instruct vcvt2Lto2F(vecD dst, vecX src, vRegF tmp) %{ predicate(n->as_Vector()->length() == 2 && n->bottom_type()->is_vect()->element_basic_type() == T_FLOAT); match(Set dst (VectorCastL2X src)); - format %{ "scvtfv T2D, $dst, $src\n\t" - "fcvtn $dst, T2S, $dst, T2D\t# convert 2L to 2F vector" + effect(TEMP_DEF dst, TEMP tmp); + format %{ "umov rscratch1, $src, D, 0\n\t" + "scvtfs $dst, rscratch1\n\t" + "umov rscratch1, $src, D, 1\n\t" + "scvtfs $tmp, rscratch1\n\t" + "ins $dst, S, $tmp, 1, 0\t# convert 2L to 2F vector" %} ins_encode %{ - __ scvtfv(__ T2D, as_FloatRegister($dst$$reg), as_FloatRegister($src$$reg)); - __ fcvtn(as_FloatRegister($dst$$reg), __ T2S, as_FloatRegister($dst$$reg), __ T2D); + __ umov(rscratch1, as_FloatRegister($src$$reg), __ D, 0); + __ scvtfs(as_FloatRegister($dst$$reg), rscratch1); + __ umov(rscratch1, as_FloatRegister($src$$reg), __ D, 1); + __ scvtfs(as_FloatRegister($tmp$$reg), rscratch1); + __ ins(as_FloatRegister($dst$$reg), __ S, as_FloatRegister($tmp$$reg), 1, 0); %} ins_pipe(pipe_slow); %} diff --git a/src/hotspot/cpu/aarch64/aarch64_neon_ad.m4 b/src/hotspot/cpu/aarch64/aarch64_neon_ad.m4 index f98ddf4ee3655..59c2cce484fa6 100644 --- a/src/hotspot/cpu/aarch64/aarch64_neon_ad.m4 +++ b/src/hotspot/cpu/aarch64/aarch64_neon_ad.m4 @@ -178,16 +178,23 @@ VECTOR_CAST_I2I_L(4, I, B, D, X, xtn, 4S, 4H, 8H, 8B) VECTOR_CAST_I2I_L(4, B, I, X, D, sxtl, 8B, 8H, 4H, 4S) dnl -instruct vcvt2Lto2F(vecD dst, vecX src) +instruct vcvt2Lto2F(vecD dst, vecX src, vRegF tmp) %{ predicate(n->as_Vector()->length() == 2 && n->bottom_type()->is_vect()->element_basic_type() == T_FLOAT); match(Set dst (VectorCastL2X src)); - format %{ "scvtfv T2D, $dst, $src\n\t" - "fcvtn $dst, T2S, $dst, T2D\t# convert 2L to 2F vector" + effect(TEMP_DEF dst, TEMP tmp); + format %{ "umov rscratch1, $src, D, 0\n\t" + "scvtfs $dst, rscratch1\n\t" + "umov rscratch1, $src, D, 1\n\t" + "scvtfs $tmp, rscratch1\n\t" + "ins $dst, S, $tmp, 1, 0\t# convert 2L to 2F vector" %} ins_encode %{ - __ scvtfv(__ T2D, as_FloatRegister($dst$$reg), as_FloatRegister($src$$reg)); - __ fcvtn(as_FloatRegister($dst$$reg), __ T2S, as_FloatRegister($dst$$reg), __ T2D); + __ umov(rscratch1, as_FloatRegister($src$$reg), __ D, 0); + __ scvtfs(as_FloatRegister($dst$$reg), rscratch1); + __ umov(rscratch1, as_FloatRegister($src$$reg), __ D, 1); + __ scvtfs(as_FloatRegister($tmp$$reg), rscratch1); + __ ins(as_FloatRegister($dst$$reg), __ S, as_FloatRegister($tmp$$reg), 1, 0); %} ins_pipe(pipe_slow); %} diff --git a/test/hotspot/jtreg/compiler/vectorapi/VectorCastShape128Test.java b/test/hotspot/jtreg/compiler/vectorapi/VectorCastShape128Test.java index 5feef36e5312f..2c97f6aa3fc28 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/VectorCastShape128Test.java +++ b/test/hotspot/jtreg/compiler/vectorapi/VectorCastShape128Test.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Arm Limited. All rights reserved. + * Copyright (c) 2021, 2022, Arm Limited. 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 @@ -114,6 +114,7 @@ public class VectorCastShape128Test { 0, Long.MAX_VALUE, Long.MIN_VALUE, + 0x561a524000000001L, }; diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX1.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX1.java index a83364fa51cd8..de70b3ca6a77b 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX1.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX1.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -30,6 +30,7 @@ /* * @test * @bug 8259610 + * @key randomness * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @summary Test that vector cast intrinsics work as intended on avx1. diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX2.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX2.java index 1b50613598b26..f05f8903355a1 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX2.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX2.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -30,6 +30,7 @@ /* * @test * @bug 8259610 + * @key randomness * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @summary Test that vector cast intrinsics work as intended on avx2. diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512.java index 749c6a21e06ef..a231889b9ed4d 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -30,6 +30,7 @@ /* * @test * @bug 8259610 + * @key randomness * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @summary Test that vector cast intrinsics work as intended on avx512. diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512BW.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512BW.java index d9fbaf92d844c..c8d907db43b06 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512BW.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512BW.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -30,6 +30,7 @@ /* * @test * @bug 8278623 + * @key randomness * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @summary Test that vector cast intrinsics work as intended on avx512bw. diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512DQ.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512DQ.java index 8799cccd91841..9d8b7db7a852b 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512DQ.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastAVX512DQ.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -30,6 +30,7 @@ /* * @test * @bug 8259610 + * @key randomness * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @summary Test that vector cast intrinsics work as intended on avx512dq. diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastNeon.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastNeon.java index 11777a3ccfcf8..1beefec112445 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastNeon.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastNeon.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -30,6 +30,7 @@ /* * @test * @bug 8259610 + * @key randomness * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @summary Test that vector cast intrinsics work as intended on neon. @@ -42,7 +43,7 @@ public static void main(String[] args) { VectorReshapeHelper.runMainHelper( TestVectorCast.class, TestCastMethods.NEON_CAST_TESTS.stream(), - "-XX:+UseNeon"); + "-XX:UseSVE=0"); } } diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastSVE.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastSVE.java index 614e8e870de43..1ab1c3ca18824 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastSVE.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorCastSVE.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -30,6 +30,7 @@ /* * @test * @bug 8259610 + * @key randomness * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @summary Test that vector cast intrinsics work as intended on sve. diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorReinterpret.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorReinterpret.java index 538736193ef25..11821a6b8ebbb 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorReinterpret.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/TestVectorReinterpret.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -35,6 +35,7 @@ /* * @test * @bug 8259610 + * @key randomness * @modules jdk.incubator.vector * @modules java.base/jdk.internal.misc * @summary Test that vector reinterpret intrinsics work as intended. diff --git a/test/hotspot/jtreg/compiler/vectorapi/reshape/utils/VectorReshapeHelper.java b/test/hotspot/jtreg/compiler/vectorapi/reshape/utils/VectorReshapeHelper.java index c769c14bde0b7..6cfd934bb276f 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/reshape/utils/VectorReshapeHelper.java +++ b/test/hotspot/jtreg/compiler/vectorapi/reshape/utils/VectorReshapeHelper.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2022, 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 @@ -37,6 +37,7 @@ import java.util.stream.Stream; import jdk.incubator.vector.*; import jdk.test.lib.Asserts; +import jdk.test.lib.Utils; public class VectorReshapeHelper { public static final int INVOCATIONS = 10_000; @@ -105,7 +106,7 @@ public static void vectorCast(VectorOperators.Conversion cop, public static void runCastHelper(VectorOperators.Conversion castOp, VectorSpecies isp, VectorSpecies osp) throws Throwable { - var random = RandomGenerator.getDefault(); + var random = Utils.getRandomInstance(); boolean isUnsignedCast = castOp.name().startsWith("ZERO"); String testMethodName = VectorSpeciesPair.makePair(isp, osp, isUnsignedCast).format(); var caller = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass(); @@ -219,7 +220,7 @@ public static void vectorExpandShrink(VectorSpecies isp, VectorSpecies isp, VectorSpecies osp) throws Throwable { - var random = RandomGenerator.getDefault(); + var random = Utils.getRandomInstance(); String testMethodName = VectorSpeciesPair.makePair(isp, osp).format(); var caller = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass(); var testMethod = MethodHandles.lookup().findStatic(caller, @@ -249,7 +250,7 @@ public static void vectorDoubleExpandShrink(VectorSpecies isp, VectorSpeci } public static void runDoubleExpandShrinkHelper(VectorSpecies isp, VectorSpecies osp) throws Throwable { - var random = RandomGenerator.getDefault(); + var random = Utils.getRandomInstance(); String testMethodName = VectorSpeciesPair.makePair(isp, osp).format(); var caller = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass(); var testMethod = MethodHandles.lookup().findStatic(caller, @@ -278,7 +279,7 @@ public static void vectorRebracket(VectorSpecies isp, VectorSpecies } public static void runRebracketHelper(VectorSpecies isp, VectorSpecies osp) throws Throwable { - var random = RandomGenerator.getDefault(); + var random = Utils.getRandomInstance(); String testMethodName = VectorSpeciesPair.makePair(isp, osp).format(); var caller = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass(); var testMethod = MethodHandles.lookup().findStatic(caller, From bf094065cf935168f950b2cb2b46fbd652c233c2 Mon Sep 17 00:00:00 2001 From: Ningsheng Jian Date: Tue, 29 Mar 2022 15:35:07 +0800 Subject: [PATCH 2/2] Add comment for special value usage. --- .../jtreg/compiler/vectorapi/VectorCastShape128Test.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/hotspot/jtreg/compiler/vectorapi/VectorCastShape128Test.java b/test/hotspot/jtreg/compiler/vectorapi/VectorCastShape128Test.java index 2c97f6aa3fc28..d086e8fed5317 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/VectorCastShape128Test.java +++ b/test/hotspot/jtreg/compiler/vectorapi/VectorCastShape128Test.java @@ -114,6 +114,8 @@ public class VectorCastShape128Test { 0, Long.MAX_VALUE, Long.MIN_VALUE, + // A special value to make sure correct rounding of + // conversion from long to float. See: JDK-8282764. 0x561a524000000001L, };