8286104: use aggressive liveness for unstable_if traps

Reviewed-by: kvn, thartmann

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 {

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,                             \

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);

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  ); }

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);

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<Node*>  _skeleton_predicate_opaqs; // List of Opaque4 nodes for the loop skeleton predicates.
   GrowableArray<Node*>  _expensive_nodes;       // List of nodes that are expensive to compute and that we'd better not let the GVN freely common
   GrowableArray<Node*>  _for_post_loop_igvn;    // List of nodes for IGVN after loop opts are over
+  GrowableArray<UnstableIfTrap*> _unstable_if_traps;        // List of ifnodes after IGVN
   GrowableArray<Node_List*> _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

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;
 }
 
 

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);
   }
 

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;

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);

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

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;
   }
 

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.");
+    }
+}

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);
+}