diff --git a/src/hotspot/share/compiler/methodLiveness.hpp b/src/hotspot/share/compiler/methodLiveness.hpp index d4926e40ca9..a8c79b53cdc 100644 --- a/src/hotspot/share/compiler/methodLiveness.hpp +++ b/src/hotspot/share/compiler/methodLiveness.hpp @@ -46,7 +46,7 @@ class MethodLivenessResult : public ResourceBitMap { {} void set_is_valid() { _is_valid = true; } - bool is_valid() { return _is_valid; } + bool is_valid() const { return _is_valid; } }; class MethodLiveness : public ResourceObj { diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index 3d12db02744..3285d8e5bda 100644 --- a/src/hotspot/share/opto/c2_globals.hpp +++ b/src/hotspot/share/opto/c2_globals.hpp @@ -416,6 +416,9 @@ "Set level of loop optimization for tier 1 compiles") \ range(5, 43) \ \ + product(bool, OptimizeUnstableIf, true, DIAGNOSTIC, \ + "Optimize UnstableIf traps") \ + \ /* controls for heat-based inlining */ \ \ develop(intx, NodeCountInliningCutoff, 18000, \ diff --git a/src/hotspot/share/opto/callnode.cpp b/src/hotspot/share/opto/callnode.cpp index b16891b560f..f03c7b52c04 100644 --- a/src/hotspot/share/opto/callnode.cpp +++ b/src/hotspot/share/opto/callnode.cpp @@ -1459,6 +1459,12 @@ Node *SafePointNode::peek_monitor_obj() const { return monitor_obj(jvms(), mon); } +Node* SafePointNode::peek_operand(uint off) const { + assert(jvms()->sp() > 0, "must have an operand"); + assert(off < jvms()->sp(), "off is out-of-range"); + return stack(jvms(), jvms()->sp() - off - 1); +} + // Do we Match on this edge index or not? Match no edges uint SafePointNode::match_edge(uint idx) const { return (TypeFunc::Parms == idx); diff --git a/src/hotspot/share/opto/callnode.hpp b/src/hotspot/share/opto/callnode.hpp index 0221b5d570d..4f354c99b0e 100644 --- a/src/hotspot/share/opto/callnode.hpp +++ b/src/hotspot/share/opto/callnode.hpp @@ -416,6 +416,8 @@ public: void pop_monitor (); Node *peek_monitor_box() const; Node *peek_monitor_obj() const; + // Peek Operand Stacks, JVMS 2.6.2 + Node* peek_operand(uint off = 0) const; // Access functions for the JVM Node *control () const { return in(TypeFunc::Control ); } diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index 42c23b40322..0c3fb5d74e2 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -396,6 +396,10 @@ void Compile::remove_useless_node(Node* dead) { remove_useless_late_inlines( &_string_late_inlines, dead); remove_useless_late_inlines( &_boxing_late_inlines, dead); remove_useless_late_inlines(&_vector_reboxing_late_inlines, dead); + + if (dead->is_CallStaticJava()) { + remove_unstable_if_trap(dead->as_CallStaticJava(), false); + } } BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2(); bs->unregister_potential_barrier_node(dead); @@ -434,6 +438,7 @@ void Compile::remove_useless_nodes(Unique_Node_List &useful) { remove_useless_nodes(_skeleton_predicate_opaqs, useful); remove_useless_nodes(_expensive_nodes, useful); // remove useless expensive nodes remove_useless_nodes(_for_post_loop_igvn, useful); // remove useless node recorded for post loop opts IGVN pass + remove_useless_unstable_if_traps(useful); // remove useless unstable_if traps remove_useless_coarsened_locks(useful); // remove useless coarsened locks nodes BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2(); @@ -607,6 +612,7 @@ Compile::Compile( ciEnv* ci_env, ciMethod* target, int osr_bci, _skeleton_predicate_opaqs (comp_arena(), 8, 0, NULL), _expensive_nodes (comp_arena(), 8, 0, NULL), _for_post_loop_igvn(comp_arena(), 8, 0, NULL), + _unstable_if_traps (comp_arena(), 8, 0, NULL), _coarsened_locks (comp_arena(), 8, 0, NULL), _congraph(NULL), NOT_PRODUCT(_igv_printer(NULL) COMMA) @@ -1854,6 +1860,106 @@ void Compile::process_for_post_loop_opts_igvn(PhaseIterGVN& igvn) { } } +void Compile::record_unstable_if_trap(UnstableIfTrap* trap) { + if (OptimizeUnstableIf) { + _unstable_if_traps.append(trap); + } +} + +void Compile::remove_useless_unstable_if_traps(Unique_Node_List& useful) { + for (int i = _unstable_if_traps.length() - 1; i >= 0; i--) { + UnstableIfTrap* trap = _unstable_if_traps.at(i); + Node* n = trap->uncommon_trap(); + if (!useful.member(n)) { + _unstable_if_traps.delete_at(i); // replaces i-th with last element which is known to be useful (already processed) + } + } +} + +// Remove the unstable if trap associated with 'unc' from candidates. It is either dead +// or fold-compares case. Return true if succeed or not found. +// +// In rare cases, the found trap has been processed. It is too late to delete it. Return +// false and ask fold-compares to yield. +// +// 'fold-compares' may use the uncommon_trap of the dominating IfNode to cover the fused +// IfNode. This breaks the unstable_if trap invariant: control takes the unstable path +// when deoptimization does happen. +bool Compile::remove_unstable_if_trap(CallStaticJavaNode* unc, bool yield) { + for (int i = 0; i < _unstable_if_traps.length(); ++i) { + UnstableIfTrap* trap = _unstable_if_traps.at(i); + if (trap->uncommon_trap() == unc) { + if (yield && trap->modified()) { + return false; + } + _unstable_if_traps.delete_at(i); + break; + } + } + return true; +} + +// Re-calculate unstable_if traps with the liveness of next_bci, which points to the unlikely path. +// It needs to be done after igvn because fold-compares may fuse uncommon_traps and before renumbering. +void Compile::process_for_unstable_if_traps(PhaseIterGVN& igvn) { + for (int i = _unstable_if_traps.length() - 1; i >= 0; --i) { + UnstableIfTrap* trap = _unstable_if_traps.at(i); + CallStaticJavaNode* unc = trap->uncommon_trap(); + int next_bci = trap->next_bci(); + bool modified = trap->modified(); + + if (next_bci != -1 && !modified) { + assert(!_dead_node_list.test(unc->_idx), "changing a dead node!"); + JVMState* jvms = unc->jvms(); + ciMethod* method = jvms->method(); + ciBytecodeStream iter(method); + + iter.force_bci(jvms->bci()); + assert(next_bci == iter.next_bci() || next_bci == iter.get_dest(), "wrong next_bci at unstable_if"); + Bytecodes::Code c = iter.cur_bc(); + Node* lhs = nullptr; + Node* rhs = nullptr; + if (c == Bytecodes::_if_acmpeq || c == Bytecodes::_if_acmpne) { + lhs = unc->peek_operand(0); + rhs = unc->peek_operand(1); + } else if (c == Bytecodes::_ifnull || c == Bytecodes::_ifnonnull) { + lhs = unc->peek_operand(0); + } + + ResourceMark rm; + const MethodLivenessResult& live_locals = method->liveness_at_bci(next_bci); + assert(live_locals.is_valid(), "broken liveness info"); + int len = (int)live_locals.size(); + + for (int i = 0; i < len; i++) { + Node* local = unc->local(jvms, i); + // kill local using the liveness of next_bci. + // give up when the local looks like an operand to secure reexecution. + if (!live_locals.at(i) && !local->is_top() && local != lhs && local!= rhs) { + uint idx = jvms->locoff() + i; +#ifdef ASSERT + if (Verbose) { + tty->print("[unstable_if] kill local#%d: ", idx); + local->dump(); + tty->cr(); + } +#endif + igvn.replace_input_of(unc, idx, top()); + modified = true; + } + } + } + + // keep the mondified trap for late query + if (modified) { + trap->set_modified(); + } else { + _unstable_if_traps.delete_at(i); + } + } + igvn.optimize(); +} + // StringOpts and late inlining of string methods void Compile::inline_string_calls(bool parse_time) { { @@ -2138,6 +2244,8 @@ void Compile::Optimize() { print_method(PHASE_ITER_GVN1, 2); + process_for_unstable_if_traps(igvn); + inline_incrementally(igvn); print_method(PHASE_INCREMENTAL_INLINE, 2); diff --git a/src/hotspot/share/opto/compile.hpp b/src/hotspot/share/opto/compile.hpp index d2f7a36852a..cca22473852 100644 --- a/src/hotspot/share/opto/compile.hpp +++ b/src/hotspot/share/opto/compile.hpp @@ -51,6 +51,7 @@ class AddPNode; class Block; class Bundle; class CallGenerator; +class CallStaticJavaNode; class CloneMap; class ConnectionGraph; class IdealGraphPrinter; @@ -90,6 +91,7 @@ class TypeOopPtr; class TypeFunc; class TypeVect; class Unique_Node_List; +class UnstableIfTrap; class nmethod; class Node_Stack; struct Final_Reshape_Counts; @@ -357,6 +359,7 @@ class Compile : public Phase { GrowableArray _skeleton_predicate_opaqs; // List of Opaque4 nodes for the loop skeleton predicates. GrowableArray _expensive_nodes; // List of nodes that are expensive to compute and that we'd better not let the GVN freely common GrowableArray _for_post_loop_igvn; // List of nodes for IGVN after loop opts are over + GrowableArray _unstable_if_traps; // List of ifnodes after IGVN GrowableArray _coarsened_locks; // List of coarsened Lock and Unlock nodes ConnectionGraph* _congraph; #ifndef PRODUCT @@ -732,6 +735,11 @@ class Compile : public Phase { void remove_from_post_loop_opts_igvn(Node* n); void process_for_post_loop_opts_igvn(PhaseIterGVN& igvn); + void record_unstable_if_trap(UnstableIfTrap* trap); + bool remove_unstable_if_trap(CallStaticJavaNode* unc, bool yield); + void remove_useless_unstable_if_traps(Unique_Node_List &useful); + void process_for_unstable_if_traps(PhaseIterGVN& igvn); + void sort_macro_nodes(); // remove the opaque nodes that protect the predicates so that the unused checks and diff --git a/src/hotspot/share/opto/graphKit.cpp b/src/hotspot/share/opto/graphKit.cpp index fe8c7242fee..984533fe9f8 100644 --- a/src/hotspot/share/opto/graphKit.cpp +++ b/src/hotspot/share/opto/graphKit.cpp @@ -2018,12 +2018,12 @@ void GraphKit::increment_counter(Node* counter_addr) { // Bail out to the interpreter in mid-method. Implemented by calling the // uncommon_trap blob. This helper function inserts a runtime call with the // right debug info. -void GraphKit::uncommon_trap(int trap_request, +Node* GraphKit::uncommon_trap(int trap_request, ciKlass* klass, const char* comment, bool must_throw, bool keep_exact_action) { if (failing()) stop(); - if (stopped()) return; // trap reachable? + if (stopped()) return NULL; // trap reachable? // Note: If ProfileTraps is true, and if a deopt. actually // occurs here, the runtime will make sure an MDO exists. There is @@ -2139,6 +2139,7 @@ void GraphKit::uncommon_trap(int trap_request, root()->add_req(halt); stop_and_kill_map(); + return call; } diff --git a/src/hotspot/share/opto/graphKit.hpp b/src/hotspot/share/opto/graphKit.hpp index 1664cbfe3f6..7ebb8b04baf 100644 --- a/src/hotspot/share/opto/graphKit.hpp +++ b/src/hotspot/share/opto/graphKit.hpp @@ -728,25 +728,25 @@ class GraphKit : public Phase { // The optional klass is the one causing the trap. // The optional reason is debug information written to the compile log. // Optional must_throw is the same as with add_safepoint_edges. - void uncommon_trap(int trap_request, + Node* uncommon_trap(int trap_request, ciKlass* klass = NULL, const char* reason_string = NULL, bool must_throw = false, bool keep_exact_action = false); // Shorthand, to avoid saying "Deoptimization::" so many times. - void uncommon_trap(Deoptimization::DeoptReason reason, + Node* uncommon_trap(Deoptimization::DeoptReason reason, Deoptimization::DeoptAction action, ciKlass* klass = NULL, const char* reason_string = NULL, bool must_throw = false, bool keep_exact_action = false) { - uncommon_trap(Deoptimization::make_trap_request(reason, action), + return uncommon_trap(Deoptimization::make_trap_request(reason, action), klass, reason_string, must_throw, keep_exact_action); } // Bail out to the interpreter and keep exact action (avoid switching to Action_none). - void uncommon_trap_exact(Deoptimization::DeoptReason reason, + Node* uncommon_trap_exact(Deoptimization::DeoptReason reason, Deoptimization::DeoptAction action, ciKlass* klass = NULL, const char* reason_string = NULL, bool must_throw = false) { - uncommon_trap(Deoptimization::make_trap_request(reason, action), + return uncommon_trap(Deoptimization::make_trap_request(reason, action), klass, reason_string, must_throw, /*keep_exact_action=*/true); } diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index ad694f07276..a7fb2e2018c 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -838,7 +838,9 @@ bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNod ciMethod* dom_method = dom_unc->jvms()->method(); int dom_bci = dom_unc->jvms()->bci(); if (!igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_unstable_fused_if) && - !igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_range_check)) { + !igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_range_check) && + // Return true if c2 manages to reconcile with UnstableIf optimization. See the comments for it. + igvn->C->remove_unstable_if_trap(dom_unc, true/*yield*/)) { success = unc_proj; fail = unc_proj->other_if_proj(); return true; diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp index 5add4786131..609b8f64130 100644 --- a/src/hotspot/share/opto/node.cpp +++ b/src/hotspot/share/opto/node.cpp @@ -665,6 +665,10 @@ void Node::destruct(PhaseValues* phase) { if (is_SafePoint()) { as_SafePoint()->delete_replaced_nodes(); + + if (is_CallStaticJava()) { + compile->remove_unstable_if_trap(as_CallStaticJava(), false); + } } BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2(); bs->unregister_potential_barrier_node(this); diff --git a/src/hotspot/share/opto/parse.hpp b/src/hotspot/share/opto/parse.hpp index 6a4d77479f6..14724ed6995 100644 --- a/src/hotspot/share/opto/parse.hpp +++ b/src/hotspot/share/opto/parse.hpp @@ -603,4 +603,41 @@ class Parse : public GraphKit { #endif }; +// Specialized uncommon_trap of unstable_if. C2 uses next_bci of path to update the live locals of it. +class UnstableIfTrap { + CallStaticJavaNode* const _unc; + bool _modified; // modified locals based on next_bci() + int _next_bci; + +public: + UnstableIfTrap(CallStaticJavaNode* call, Parse::Block* path): _unc(call), _modified(false) { + assert(_unc != NULL && Deoptimization::trap_request_reason(_unc->uncommon_trap_request()) == Deoptimization::Reason_unstable_if, + "invalid uncommon_trap call!"); + _next_bci = path != nullptr ? path->start() : -1; + } + + // The starting point of the pruned block, where control goes when + // deoptimization does happen. + int next_bci() const { + return _next_bci; + } + + bool modified() const { + return _modified; + } + + void set_modified() { + _modified = true; + } + + CallStaticJavaNode* uncommon_trap() const { + return _unc; + } + + inline void* operator new(size_t x) throw() { + Compile* C = Compile::current(); + return C->comp_arena()->AmallocWords(x); + } +}; + #endif // SHARE_OPTO_PARSE_HPP diff --git a/src/hotspot/share/opto/parse2.cpp b/src/hotspot/share/opto/parse2.cpp index ab3b68515c5..d923dfb196b 100644 --- a/src/hotspot/share/opto/parse2.cpp +++ b/src/hotspot/share/opto/parse2.cpp @@ -1586,10 +1586,14 @@ void Parse::adjust_map_after_if(BoolTest::mask btest, Node* c, float prob, if (path_is_suitable_for_uncommon_trap(prob)) { repush_if_args(); - uncommon_trap(Deoptimization::Reason_unstable_if, + Node* call = uncommon_trap(Deoptimization::Reason_unstable_if, Deoptimization::Action_reinterpret, NULL, (is_fallthrough ? "taken always" : "taken never")); + + if (call != nullptr) { + C->record_unstable_if_trap(new UnstableIfTrap(call->as_CallStaticJava(), path)); + } return; } diff --git a/test/hotspot/jtreg/compiler/c2/TestFoldCompares2.java b/test/hotspot/jtreg/compiler/c2/TestFoldCompares2.java new file mode 100644 index 00000000000..b24e49b0b31 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/TestFoldCompares2.java @@ -0,0 +1,82 @@ +/* + * Copyright Amazon.com Inc. 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 + * @bug 8286104 + * @summary Test Fold-compares are safe when C2 optimizes unstable_if traps + * (-XX:+OptimizeUnstableIf) + * + * @run main/othervm -XX:CompileCommand=compileOnly,java.lang.Short::valueOf + * -XX:CompileCommand=compileonly,compiler.c2.TestFoldCompares2$Numbers::isSupported + * -Xbatch compiler.c2.TestFoldCompares2 + */ + +package compiler.c2; + +public class TestFoldCompares2 { + public static Short value = Short.valueOf((short) 0); + static void testShort() { + // trigger compilation and bias to a cached value. + for (int i=0; i<20_000; ++i) { + value = Short.valueOf((short) 0); + } + + // trigger deoptimization on purpose + // the size of ShortCache.cache is hard-coded in java.lang.Short + Short x = Short.valueOf((short) 128); + if (x != 128) { + throw new RuntimeException("wrong result!"); + } + } + + static enum Numbers { + One, + Two, + Three, + Four, + Five; + + boolean isSupported() { + // ordinal() is inlined and leaves a copy region node, which blocks + // fold-compares in the 1st iterGVN. + return ordinal() >= Two.ordinal() && ordinal() <= Four.ordinal(); + } + } + + static void testEnumValues() { + Numbers local = Numbers.Two; + + for (int i = 0; i < 2_000_000; ++i) { + local.isSupported(); + } + // deoptimize + Numbers.Five.isSupported(); + } + + public static void main(String[] args) { + testShort(); + testEnumValues(); + System.out.println("Test passed."); + } +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/TestOptimizeUnstableIf.java b/test/hotspot/jtreg/compiler/c2/irTests/TestOptimizeUnstableIf.java new file mode 100644 index 00000000000..ea64b43908a --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/TestOptimizeUnstableIf.java @@ -0,0 +1,73 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +package compiler.c2.irTests; + +import jdk.test.lib.Asserts; +import compiler.lib.ir_framework.*; + +/* + * @test + * @bug 8286104 + * @summary Test that C2 uses aggressive liveness to get rid of the boxing object which is + * only consumed by uncommon_trap. + * @library /test/lib / + * @run driver compiler.c2.irTests.TestOptimizeUnstableIf + */ +public class TestOptimizeUnstableIf { + + public static void main(String[] args) { + TestFramework.run(); + } + + @Test + @Arguments({Argument.MAX}) // the argument needs to be big enough to fall out of cache. + @IR(failOn = {IRNode.ALLOC_OF, "Integer"}) + public static int boxing_object(int value) { + Integer ii = Integer.valueOf(value); + int sum = 0; + + if (value > 999_999) { + sum += ii.intValue(); + } + + return sum; + } + + @Check(test = "boxing_object") + public void checkWithTestInfo(int result, TestInfo info) { + if (info.isWarmUp()) { + // Accessing the cached boxing object during warm-up phase. It prevents parser from pruning that branch of Interger.valueOf(); + // This guarantees that a phi node is generated, which merge a cached object and the newly allocated object. eg. + // 112: Phi === 108 168 188 [[ 50 ]] #java/lang/Integer:NotNull:exact * Oop:java/lang/Integer:NotNull:exact * + // 168: a cached object + // 188: result of AllocateNode + // 50: uncommon_trap unstable_if + value += Integer.valueOf(0); + } + + Asserts.assertEQ(result, Integer.MAX_VALUE); + } + + public static Integer value = Integer.valueOf(0); +}