From 82afab44e4419d60e144f0a03309c747e0fb9917 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Wed, 26 Mar 2025 13:23:12 +0100 Subject: [PATCH] Revert "8302459: Missing late inline cleanup causes compiler/vectorapi/VectorLogicalOpIdentityTest.java IR failure" This reverts commit 2e4d7d1846d846fd98201b9b3abeb7b91239a40d. --- src/hotspot/share/opto/callGenerator.cpp | 2 +- src/hotspot/share/opto/callnode.cpp | 119 +++++++----------- src/hotspot/share/opto/callnode.hpp | 1 - src/hotspot/share/opto/compile.cpp | 11 -- test/hotspot/jtreg/ProblemList.txt | 1 + .../VectorGatherMaskFoldingTest.java | 11 +- .../VectorLogicalOpIdentityTest.java | 5 +- 7 files changed, 54 insertions(+), 96 deletions(-) diff --git a/src/hotspot/share/opto/callGenerator.cpp b/src/hotspot/share/opto/callGenerator.cpp index 5f010d5c42799..ec7117e3568ca 100644 --- a/src/hotspot/share/opto/callGenerator.cpp +++ b/src/hotspot/share/opto/callGenerator.cpp @@ -470,7 +470,7 @@ class LateInlineVirtualCallGenerator : public VirtualCallGenerator { virtual void do_late_inline(); virtual void set_callee_method(ciMethod* m) { - assert(_callee == nullptr || _callee == m, "repeated inline attempt with different callee"); + assert(_callee == nullptr, "repeated inlining attempt"); _callee = m; } diff --git a/src/hotspot/share/opto/callnode.cpp b/src/hotspot/share/opto/callnode.cpp index bb30dcb2976a7..68efabea15cf7 100644 --- a/src/hotspot/share/opto/callnode.cpp +++ b/src/hotspot/share/opto/callnode.cpp @@ -1091,15 +1091,6 @@ void CallJavaNode::dump_compact_spec(outputStream* st) const { } #endif -void CallJavaNode::register_for_late_inline() { - if (generator() != nullptr) { - Compile::current()->prepend_late_inline(generator()); - set_generator(nullptr); - } else { - assert(false, "repeated inline attempt"); - } -} - //============================================================================= uint CallStaticJavaNode::size_of() const { return sizeof(*this); } bool CallStaticJavaNode::cmp( const Node &n ) const { @@ -1110,35 +1101,26 @@ bool CallStaticJavaNode::cmp( const Node &n ) const { Node* CallStaticJavaNode::Ideal(PhaseGVN* phase, bool can_reshape) { CallGenerator* cg = generator(); if (can_reshape && cg != nullptr) { - if (cg->is_mh_late_inline()) { - assert(IncrementalInlineMH, "required"); - assert(cg->call_node() == this, "mismatch"); - assert(cg->method()->is_method_handle_intrinsic(), "required"); - - // Check whether this MH handle call becomes a candidate for inlining. - ciMethod* callee = cg->method(); - vmIntrinsics::ID iid = callee->intrinsic_id(); - if (iid == vmIntrinsics::_invokeBasic) { - if (in(TypeFunc::Parms)->Opcode() == Op_ConP) { - register_for_late_inline(); - } - } else if (iid == vmIntrinsics::_linkToNative) { - // never retry - } else { - assert(callee->has_member_arg(), "wrong type of call?"); - if (in(TypeFunc::Parms + callee->arg_size() - 1)->Opcode() == Op_ConP) { - register_for_late_inline(); - phase->C->inc_number_of_mh_late_inlines(); - } + assert(IncrementalInlineMH, "required"); + assert(cg->call_node() == this, "mismatch"); + assert(cg->is_mh_late_inline(), "not virtual"); + + // Check whether this MH handle call becomes a candidate for inlining. + ciMethod* callee = cg->method(); + vmIntrinsics::ID iid = callee->intrinsic_id(); + if (iid == vmIntrinsics::_invokeBasic) { + if (in(TypeFunc::Parms)->Opcode() == Op_ConP) { + phase->C->prepend_late_inline(cg); + set_generator(nullptr); } + } else if (iid == vmIntrinsics::_linkToNative) { + // never retry } else { - assert(IncrementalInline, "required"); - assert(!cg->method()->is_method_handle_intrinsic(), "required"); - if (phase->C->print_inlining()) { - phase->C->inline_printer()->record(cg->method(), cg->call_node()->jvms(), InliningResult::FAILURE, - "static call node changed: trying again"); + assert(callee->has_member_arg(), "wrong type of call?"); + if (in(TypeFunc::Parms + callee->arg_size() - 1)->Opcode() == Op_ConP) { + phase->C->prepend_late_inline(cg); + set_generator(nullptr); } - register_for_late_inline(); } } return CallNode::Ideal(phase, can_reshape); @@ -1207,46 +1189,39 @@ bool CallDynamicJavaNode::cmp( const Node &n ) const { Node* CallDynamicJavaNode::Ideal(PhaseGVN* phase, bool can_reshape) { CallGenerator* cg = generator(); if (can_reshape && cg != nullptr) { - if (cg->is_virtual_late_inline()) { - assert(IncrementalInlineVirtual, "required"); - assert(cg->call_node() == this, "mismatch"); - - // Recover symbolic info for method resolution. - ciMethod* caller = jvms()->method(); - ciBytecodeStream iter(caller); - iter.force_bci(jvms()->bci()); - - bool not_used1; - ciSignature* not_used2; - ciMethod* orig_callee = iter.get_method(not_used1, ¬_used2); // callee in the bytecode - ciKlass* holder = iter.get_declared_method_holder(); - if (orig_callee->is_method_handle_intrinsic()) { - assert(_override_symbolic_info, "required"); - orig_callee = method(); - holder = method()->holder(); - } + assert(IncrementalInlineVirtual, "required"); + assert(cg->call_node() == this, "mismatch"); + assert(cg->is_virtual_late_inline(), "not virtual"); + + // Recover symbolic info for method resolution. + ciMethod* caller = jvms()->method(); + ciBytecodeStream iter(caller); + iter.force_bci(jvms()->bci()); + + bool not_used1; + ciSignature* not_used2; + ciMethod* orig_callee = iter.get_method(not_used1, ¬_used2); // callee in the bytecode + ciKlass* holder = iter.get_declared_method_holder(); + if (orig_callee->is_method_handle_intrinsic()) { + assert(_override_symbolic_info, "required"); + orig_callee = method(); + holder = method()->holder(); + } - ciInstanceKlass* klass = ciEnv::get_instance_klass_for_declared_method_holder(holder); + ciInstanceKlass* klass = ciEnv::get_instance_klass_for_declared_method_holder(holder); - Node* receiver_node = in(TypeFunc::Parms); - const TypeOopPtr* receiver_type = phase->type(receiver_node)->isa_oopptr(); + Node* receiver_node = in(TypeFunc::Parms); + const TypeOopPtr* receiver_type = phase->type(receiver_node)->isa_oopptr(); - int not_used3; - bool call_does_dispatch; - ciMethod* callee = phase->C->optimize_virtual_call(caller, klass, holder, orig_callee, receiver_type, true /*is_virtual*/, - call_does_dispatch, not_used3); // out-parameters - if (!call_does_dispatch) { - // Register for late inlining. - cg->set_callee_method(callee); - register_for_late_inline(); // MH late inlining prepends to the list, so do the same - } - } else { - assert(IncrementalInline, "required"); - if (phase->C->print_inlining()) { - phase->C->inline_printer()->record(cg->method(), cg->call_node()->jvms(), InliningResult::FAILURE, - "dynamic call node changed: trying again"); - } - register_for_late_inline(); + int not_used3; + bool call_does_dispatch; + ciMethod* callee = phase->C->optimize_virtual_call(caller, klass, holder, orig_callee, receiver_type, true /*is_virtual*/, + call_does_dispatch, not_used3); // out-parameters + if (!call_does_dispatch) { + // Register for late inlining. + cg->set_callee_method(callee); + phase->C->prepend_late_inline(cg); // MH late inlining prepends to the list, so do the same + set_generator(nullptr); } } return CallNode::Ideal(phase, can_reshape); diff --git a/src/hotspot/share/opto/callnode.hpp b/src/hotspot/share/opto/callnode.hpp index db857d4c6d1a6..fc03ab5163bb1 100644 --- a/src/hotspot/share/opto/callnode.hpp +++ b/src/hotspot/share/opto/callnode.hpp @@ -790,7 +790,6 @@ class CallJavaNode : public CallNode { void set_arg_escape(bool f) { _arg_escape = f; } bool arg_escape() const { return _arg_escape; } void copy_call_debug_info(PhaseIterGVN* phase, SafePointNode *sfpt); - void register_for_late_inline(); DEBUG_ONLY( bool validate_symbolic_info() const; ) diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index 8a059d0852f7f..56f57793fbb1e 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -2085,7 +2085,6 @@ bool Compile::inline_incrementally_one() { for (int i = 0; i < _late_inlines.length(); i++) { _late_inlines_pos = i+1; CallGenerator* cg = _late_inlines.at(i); - bool is_scheduled_for_igvn_before = C->igvn_worklist()->member(cg->call_node()); bool does_dispatch = cg->is_virtual_late_inline() || cg->is_mh_late_inline(); if (inlining_incrementally() || does_dispatch) { // a call can be either inlined or strength-reduced to a direct call cg->do_late_inline(); @@ -2096,16 +2095,6 @@ bool Compile::inline_incrementally_one() { _late_inlines_pos = i+1; // restore the position in case new elements were inserted print_method(PHASE_INCREMENTAL_INLINE_STEP, 3, cg->call_node()); break; // process one call site at a time - } else { - bool is_scheduled_for_igvn_after = C->igvn_worklist()->member(cg->call_node()); - if (!is_scheduled_for_igvn_before && is_scheduled_for_igvn_after) { - // Avoid potential infinite loop if node already in the IGVN list - assert(false, "scheduled for IGVN during inlining attempt"); - } else { - // Ensure call node has not disappeared from IGVN worklist during a failed inlining attempt - assert(!is_scheduled_for_igvn_before || is_scheduled_for_igvn_after, "call node removed from IGVN list during inlining pass"); - cg->call_node()->set_generator(cg); - } } } else { // Ignore late inline direct calls when inlining is not allowed. diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt index d76f4b2d95b66..6dd35997caca3 100644 --- a/test/hotspot/jtreg/ProblemList.txt +++ b/test/hotspot/jtreg/ProblemList.txt @@ -56,6 +56,7 @@ compiler/c2/irTests/TestDuplicateBackedge.java 8318904 generic-all compiler/codecache/jmx/PoolsIndependenceTest.java 8264632 macosx-all compiler/vectorapi/reshape/TestVectorReinterpret.java 8320897,8348519 aix-ppc64,linux-ppc64le,linux-s390x +compiler/vectorapi/VectorLogicalOpIdentityTest.java 8302459 linux-x64,windows-x64 compiler/vectorapi/VectorRebracket128Test.java 8330538 generic-all compiler/jvmci/TestUncaughtErrorInCompileMethod.java 8309073 generic-all diff --git a/test/hotspot/jtreg/compiler/vectorapi/VectorGatherMaskFoldingTest.java b/test/hotspot/jtreg/compiler/vectorapi/VectorGatherMaskFoldingTest.java index 7e16c5e5e8645..88175baf8e4a6 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/VectorGatherMaskFoldingTest.java +++ b/test/hotspot/jtreg/compiler/vectorapi/VectorGatherMaskFoldingTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 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 @@ -32,7 +32,7 @@ /** * @test - * @bug 8325520 8302459 + * @bug 8325520 * @library /test/lib / * @summary Don't allow folding of Load/Store vectors when using incompatible indices or masks * @modules jdk.incubator.vector @@ -1398,12 +1398,7 @@ public static void testFloatVectorLoadMaskedStoreVector() { public static void main(String[] args) { TestFramework testFramework = new TestFramework(); testFramework.setDefaultWarmup(10000) - .addFlags("--add-modules=jdk.incubator.vector") + .addFlags("--add-modules=jdk.incubator.vector", "-XX:+IgnoreUnrecognizedVMOptions", "-XX:+IncrementalInlineForceCleanup") .start(); - testFramework = new TestFramework(); - testFramework.setDefaultWarmup(10000) - .addFlags("--add-modules=jdk.incubator.vector", "-XX:-TieredCompilation") - .start(); - } } diff --git a/test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java b/test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java index 1a2402222ed84..426dec6701933 100644 --- a/test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java +++ b/test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java @@ -1,6 +1,6 @@ /* * Copyright (c) 2022, 2023, Arm Limited. All rights reserved. - * Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 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 @@ -41,7 +41,7 @@ /** * @test - * @bug 8288294 8302459 + * @bug 8288294 * @key randomness * @library /test/lib / * @summary Add identity transformations for vector logic operations @@ -761,6 +761,5 @@ public static void testMaskXorSame() { public static void main(String[] args) { TestFramework.runWithFlags("--add-modules=jdk.incubator.vector"); - TestFramework.runWithFlags("--add-modules=jdk.incubator.vector", "-XX:-TieredCompilation"); } }