From 42439eb60c4488711f182d0d6ee5165b4972b99d Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Tue, 20 Jan 2026 18:30:42 +0000 Subject: [PATCH] 8374889: C2 VectorAPI: must handle impossible combination of signed cast from float Reviewed-by: dlong, qamai --- src/hotspot/share/opto/graphKit.cpp | 23 +++-- src/hotspot/share/opto/graphKit.hpp | 2 + src/hotspot/share/opto/parse1.cpp | 3 +- src/hotspot/share/opto/vectorIntrinsics.cpp | 18 +++- .../vectorapi/TestCastShapeBadOpc.java | 91 +++++++++++++++++++ 5 files changed, 127 insertions(+), 10 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/vectorapi/TestCastShapeBadOpc.java diff --git a/src/hotspot/share/opto/graphKit.cpp b/src/hotspot/share/opto/graphKit.cpp index 71ae8fe44a7..bc8ebaf1869 100644 --- a/src/hotspot/share/opto/graphKit.cpp +++ b/src/hotspot/share/opto/graphKit.cpp @@ -1489,8 +1489,7 @@ Node* GraphKit::must_be_not_null(Node* value, bool do_replace_in_map) { } Node *if_f = _gvn.transform(new IfFalseNode(iff)); Node *frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); - Node* halt = _gvn.transform(new HaltNode(if_f, frame, "unexpected null in intrinsic")); - C->root()->add_req(halt); + halt(if_f, frame, "unexpected null in intrinsic"); Node *if_t = _gvn.transform(new IfTrueNode(iff)); set_control(if_t); return cast_not_null(value, do_replace_in_map); @@ -2073,6 +2072,12 @@ void GraphKit::increment_counter(Node* counter_addr) { store_to_memory(ctrl, counter_addr, incr, T_LONG, MemNode::unordered); } +void GraphKit::halt(Node* ctrl, Node* frameptr, const char* reason, bool generate_code_in_product) { + Node* halt = new HaltNode(ctrl, frameptr, reason + PRODUCT_ONLY(COMMA generate_code_in_product)); + halt = _gvn.transform(halt); + root()->add_req(halt); +} //------------------------------uncommon_trap---------------------------------- // Bail out to the interpreter in mid-method. Implemented by calling the @@ -2195,11 +2200,15 @@ Node* GraphKit::uncommon_trap(int trap_request, // The debug info is the only real input to this call. // Halt-and-catch fire here. The above call should never return! - HaltNode* halt = new HaltNode(control(), frameptr(), "uncommon trap returned which should never happen" - PRODUCT_ONLY(COMMA /*reachable*/false)); - _gvn.set_type_bottom(halt); - root()->add_req(halt); - + // We only emit code for the HaltNode in debug, which is enough for + // verifying correctness. In product, we don't want to emit it so + // that we can save on code space. HaltNode often get folded because + // the compiler can prove that the unreachable path is dead. But we + // cannot generally expect that for uncommon traps, which are often + // reachable and occasionally taken. + halt(control(), frameptr(), + "uncommon trap returned which should never happen", + false /* don't emit code in product */); stop_and_kill_map(); return call; } diff --git a/src/hotspot/share/opto/graphKit.hpp b/src/hotspot/share/opto/graphKit.hpp index 56e9a949e6f..0537d31ae36 100644 --- a/src/hotspot/share/opto/graphKit.hpp +++ b/src/hotspot/share/opto/graphKit.hpp @@ -709,6 +709,8 @@ class GraphKit : public Phase { void increment_counter(address counter_addr); // increment a debug counter void increment_counter(Node* counter_addr); // increment a debug counter + void halt(Node* ctrl, Node* frameptr, const char* reason, bool generate_code_in_product = true); + // Bail out to the interpreter right now // The optional klass is the one causing the trap. // The optional reason is debug information written to the compile log. diff --git a/src/hotspot/share/opto/parse1.cpp b/src/hotspot/share/opto/parse1.cpp index 7aa96d2ace3..6122f7e7bfc 100644 --- a/src/hotspot/share/opto/parse1.cpp +++ b/src/hotspot/share/opto/parse1.cpp @@ -1229,8 +1229,7 @@ void Parse::do_method_entry() { Node* not_subtype_ctrl = gen_subtype_check(receiver_obj, holder_klass); assert(!stopped(), "not a subtype"); - Node* halt = _gvn.transform(new HaltNode(not_subtype_ctrl, frameptr(), "failed receiver subtype check")); - C->root()->add_req(halt); + halt(not_subtype_ctrl, frameptr(), "failed receiver subtype check"); } } #endif // ASSERT diff --git a/src/hotspot/share/opto/vectorIntrinsics.cpp b/src/hotspot/share/opto/vectorIntrinsics.cpp index b48b5f2cd05..6dcf4615b10 100644 --- a/src/hotspot/share/opto/vectorIntrinsics.cpp +++ b/src/hotspot/share/opto/vectorIntrinsics.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2026, 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 @@ -25,6 +25,7 @@ #include "ci/ciSymbols.hpp" #include "classfile/vmSymbols.hpp" #include "opto/library_call.hpp" +#include "opto/rootnode.hpp" #include "opto/runtime.hpp" #include "opto/vectornode.hpp" #include "prims/vectorSupport.hpp" @@ -2330,6 +2331,21 @@ bool LibraryCallKit::inline_vector_convert() { Node* op = opd1; if (is_cast) { assert(!is_mask || num_elem_from == num_elem_to, "vector mask cast needs the same elem num"); + + // Make sure the precondition of VectorCastNode::opcode holds: we can only have + // unsigned casts for integral types (excluding long). VectorAPI code is not + // expected to violate this at runtime, but we may compile unreachable code + // where such impossible combinations arise. + if (is_ucast && (!is_integral_type(elem_bt_from) || elem_bt_from == T_LONG)) { + // Halt-and-catch fire here. This condition should never happen at runtime. + stringStream ss; + ss.print("impossible combination: unsigned vector cast from %s", type2name(elem_bt_from)); + halt(control(), frameptr(), ss.as_string(C->comp_arena())); + stop_and_kill_map(); + log_if_needed(" ** impossible combination: unsigned cast from %s", type2name(elem_bt_from)); + return true; + } + int cast_vopc = VectorCastNode::opcode(-1, elem_bt_from, !is_ucast); // Make sure that vector cast is implemented to particular type/size combination if it is diff --git a/test/hotspot/jtreg/compiler/vectorapi/TestCastShapeBadOpc.java b/test/hotspot/jtreg/compiler/vectorapi/TestCastShapeBadOpc.java new file mode 100644 index 00000000000..4c20c84bc50 --- /dev/null +++ b/test/hotspot/jtreg/compiler/vectorapi/TestCastShapeBadOpc.java @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2026, 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 + * 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. + */ + +/* + * @test id=all-flags + * @bug 8374889 + * @summary Test case that can compile unexpected code paths in VectorAPI cast intrinsification. + * @modules jdk.incubator.vector + * @library /test/lib / + * @run main/othervm + * -XX:+IgnoreUnrecognizedVMOptions + * -XX:-TieredCompilation -Xbatch + * -XX:StressSeed=1462975402 + * -XX:+StressIncrementalInlining + * -XX:CompileCommand=compileonly,${test.main.class}::test2 + * ${test.main.class} + */ + +/* + * @test id=no-stress-seed + * @bug 8374889 + * @modules jdk.incubator.vector + * @library /test/lib / + * @run main/othervm + * -XX:+IgnoreUnrecognizedVMOptions + * -XX:-TieredCompilation -Xbatch + * -XX:+StressIncrementalInlining + * -XX:CompileCommand=compileonly,${test.main.class}::test2 + * ${test.main.class} + */ + +/* + * @test id=vanilla + * @bug 8374889 + * @modules jdk.incubator.vector + * @library /test/lib / + * @run driver ${test.main.class} + */ + +package compiler.vectorapi; + +import jdk.incubator.vector.*; + +public class TestCastShapeBadOpc { + public static void main(String[] args) { + for (int i = 0; i < 100_000; ++i) { + test1(); + test2(); + } + } + + // This code does not trigger the bug itself, but seems to be important for profiling, + // so that test2 fails. + public static Object test1() { + LongVector v0 = LongVector.broadcast(LongVector.SPECIES_512, -15L); + var v1 = (ByteVector)v0.convertShape(VectorOperators.Conversion.ofReinterpret(long.class, byte.class), ByteVector.SPECIES_128, 0); + var v2 = (ByteVector)v1.castShape(ByteVector.SPECIES_256, 0); + return v2; + } + + public static Object test2() { + var v0 = ShortVector.broadcast(ShortVector.SPECIES_64, (short)7729); + var v1 = (FloatVector)v0.reinterpretShape(FloatVector.SPECIES_64, 0); + // The castShape below should take the "C" path in AbstractVector::convert0, but sometimes + // we also compile the "Z" case because of profiling. This means we attempt to create + // a vector cast from float -> long, but unfortunately with a UCAST (float -> long is signed). + // This triggered an assert in VectorCastNode::opcode. + var v2 = (LongVector)v1.castShape(LongVector.SPECIES_256, 0); + return v2; + } +}