JVM-1738: Incorrect jvms when PEA materializes an object in merge_common (openjdk#7)

* JVM-1738: Incorrect jvms when PEA materializes an object in merge_common

This patch copys all debuginfo nodes from the original AllocateNode and clones its JVMState as well.
We deploy a uncommon_trap rather than generating the exception for real. That shouluhd resolve JVM-1730.

The reason we give up the idea to use add_safepoint_edges() is that GraphKit's bci may point
to an arbitary bytecode and sp even doesn't match its current bci.

The another benefit is that it can mimic the original AllocateNode with the exactly same debuginfo and JVMState.
If OOME does emerge, the stacktrace is as if we doesn't move AllocateNode ever.

* Update passive materialization.

* Add a test for JVM-1738

---------

Co-authored-by: Xin Liu <xxinliu@amazon.com>

Author: navyxliu

PEA/CrazyException.java

@@ -0,0 +1,28 @@
+class CrazyException {
+    void blackhole() {}
+    public static Object foo(boolean cond) {
+        var x = new CrazyException();
+        for (int i=0; i< 100; ++i) {
+          if (cond)  {
+              try {
+                  x.blackhole(); // not inline. escape here.
+              } finally {        // hidden catch(Throwable) is here!
+                  close();
+              }
+          }
+        }
+        return x;
+    }
+    public static void close() {}
+    public static void main(String[] args)  {
+        int iterations = 0;
+        try {
+            while (iterations < 20_000) {
+                CrazyException.foo(0 == (iterations & 0xf));
+                iterations++;
+            }
+        } finally {
+            System.err.println("Epsilon Test: " + iterations);
+        }
+    }
+}

PEA/auto.sh

@@ -12,3 +12,4 @@ JVM_FLAGS="-XX:+PrintEscapeAnalysis -XX:+PrintEliminateAllocations -XX:+PrintOpt
 ./run3_2.sh -Xlog:gc -XX:+DoPartialEscapeAnalysis $JVM_FLAGS
 ./run3.sh -Xlog:gc -XX:+DoPartialEscapeAnalysis $JVM_FLAGS
 ./run_str.sh  -Xlog:gc -XX:+DoPartialEscapeAnalysis -XX:-UseTLAB  $JVM_FLAGS
+./run_exception.sh -Xlog:gc -XX:+DoPartialEscapeAnalysis $JVM_FLAGS

PEA/run_exception.sh

@@ -0,0 +1,2 @@
+#!/usr/bin/bash
+java -Xms16M -Xmx16M -XX:+AlwaysPreTouch -XX:+UnlockExperimentalVMOptions -XX:-TieredCompilation -XX:+UseEpsilonGC -XX:-UseOnStackReplacement -XX:CompileCommand=dontinline,CrazyException.blackhole -XX:CompileCommand=compileonly,CrazyException.foo -XX:CompileCommand=dontinline,CrazyException.close $* CrazyException

src/hotspot/share/opto/doCall.cpp

@@ -668,7 +668,7 @@ void Parse::do_call() {
         AllocateNode* alloc = state.is_alias(arg);
 
        if (alloc != nullptr && state.get_object_state(alloc)->is_virtual()) {
-         EscapedState* es = state.materialize(this, arg, map());
+         EscapedState* es = state.materialize(this, arg);
          Node* objx = es->get_materialized_value();
          set_argument(i, objx);
          AllocateNode* allocx = objx->in(1)->in(0)->as_Allocate();

src/hotspot/share/opto/graphKit.cpp

@@ -3533,14 +3533,10 @@ static void hook_memory_on_init(GraphKit& kit, int alias_idx,
   init_in_merge->set_memory_at(alias_idx, prevmem);
   kit.set_memory(init_out_raw, alias_idx);
 }
-
-//---------------------------set_output_for_allocation-------------------------
-Node* GraphKit::set_output_for_allocation(AllocateNode* alloc,
+Node* GraphKit::set_output_for_allocation_common(AllocateNode* alloc,
                                           const TypeOopPtr* oop_type,
                                           bool deoptimize_on_exception) {
   int rawidx = Compile::AliasIdxRaw;
-  alloc->set_req( TypeFunc::FramePtr, frameptr() );
-  add_safepoint_edges(alloc);
   Node* allocx = _gvn.transform(alloc);
   set_control( _gvn.transform(new ProjNode(allocx, TypeFunc::Control) ) );
   // create memory projection for i_o
@@ -3619,6 +3615,43 @@ Node* GraphKit::set_output_for_allocation(AllocateNode* alloc,
   return javaoop;
 }
 
+//---------------------------set_output_for_allocation-------------------------
+Node* GraphKit::set_output_for_allocation(AllocateNode* alloc,
+                                          const TypeOopPtr* oop_type,
+                                          bool deoptimize_on_exception) {
+  alloc->set_req( TypeFunc::FramePtr, frameptr() );
+  add_safepoint_edges(alloc);
+  return set_output_for_allocation_common(alloc, oop_type, deoptimize_on_exception);
+}
+
+
+Node* GraphKit::materialize_object(AllocateNode* alloc, const TypeOopPtr* oop_type) {
+  Node *mem = reset_memory();
+  set_all_memory(mem); // Create new memory state
+  AllocateNode* allocx  = new AllocateNode(C, alloc->tf(), control(), mem, i_o(),
+                                           alloc->in(AllocateNode::AllocSize),
+                                           alloc->in(AllocateNode::KlassNode),
+                                           alloc->in(AllocateNode::InitialTest));
+
+  allocx->set_req(TypeFunc::FramePtr, frameptr());
+
+  JVMState* out_jvms = alloc->jvms()->clone_deep(C);
+  allocx->set_jvms(out_jvms);
+  // same safepoint as original alloc
+  for (uint i=allocx->req(); i < alloc->req(); ++i) {
+    allocx->add_req(alloc->in(i));
+  }
+
+  while (out_jvms != nullptr) {
+    out_jvms->set_map(allocx);
+    out_jvms = out_jvms->caller();
+  }
+  int saved_bci = bci();
+  set_bci(alloc->jvms()->bci()); // reset to bytecode new because uncommon_trap checks bc() and sp
+  Node* objx = set_output_for_allocation_common(allocx, oop_type, true /*deoptimize_on_ex*/);
+  set_bci(saved_bci);
+  return objx;
+}
 //---------------------------new_instance--------------------------------------
 // This routine takes a klass_node which may be constant (for a static type)
 // or may be non-constant (for reflective code).  It will work equally well

src/hotspot/share/opto/graphKit.hpp

@@ -850,10 +850,17 @@ class GraphKit : public Phase {
   Node* subtype_check_receiver(Node* receiver, ciKlass* klass,
                                Node** casted_receiver);
 
+  Node* set_output_for_allocation_common(AllocateNode* alloc,
+                                  const TypeOopPtr* oop_type,
+                                  bool deoptimize_on_exception);
+
   // implementation of object creation
   Node* set_output_for_allocation(AllocateNode* alloc,
                                   const TypeOopPtr* oop_type,
                                   bool deoptimize_on_exception=false);
+
+  Node* materialize_object(AllocateNode* alloc, const TypeOopPtr* oop_type);
+
   Node* get_layout_helper(Node* klass_node, jint& constant_value);
   Node* new_instance(Node* klass_node,
                      Node* slow_test = NULL,

src/hotspot/share/opto/parse1.cpp

@@ -1781,7 +1781,8 @@ void Parse::merge_common(Parse::Block* target, int pnum) {
     // Update all the non-control inputs to map:
     assert(TypeFunc::Parms == newin->jvms()->locoff(), "parser map should contain only youngest jvms");
     bool check_elide_phi = target->is_SEL_backedge(save_block);
-    // from right to left, we update I_O and memory last because PEA materialization may change them.
+    // from right to left, we update ctrl, I_O and memory til the last minute
+    // because PEA materialization may change them.
     for (uint j = newin->req()-1; j >= 1; --j) {
       Node* m = map()->in(j);   // Current state of target.
       Node* n = newin->in(j);   // Incoming change to target state.
@@ -1829,10 +1830,8 @@ void Parse::merge_common(Parse::Block* target, int pnum) {
                     if (!pred_os->is_virtual()) {
                       // materialize 'm' because it has been materialized in save_block.
                       assert(pred_os->get_materialized_value() == n, "sanity: materialized value in save_block is n");
-                      SafePointNode* map = block()->from_block()->start_map();
-                      // object materialization takes place in from_block. It is the initial block that the current block(target) merged.
-                      // from block isn't save_block! see Block::record_state().
-                      Node* mv = ensure_object_materialized(m, map->jvms()->alloc_state(), map, r, block()->init_pnum());
+                      as.add_alias(id, m);
+                      Node* mv = ensure_object_materialized(m, as, map(), r, block()->init_pnum());
                       phi->replace_edge(m, mv);
                       as.update(id, new EscapedState(phi));
                     } else {
@@ -2174,16 +2173,50 @@ PhiNode *Parse::ensure_memory_phi(int idx, bool nocreate) {
   return phi;
 }
 
+// Passive Materialization
+// ------------------------
+// C2 has to materialize a virtual object at the merging point because the object has been materialized in
+// any other predecessor. For instance:
+//
+// obj(virtual)     obj(materialized)
+//              \      /
+// Reion(,c1,c2) |    |
+//      \        |    |
+//       \c      |    |
+//   o3 = Phi   (o1, o2)
+//
+// Node* var is o1;
+// r[pnum] is c1;
+//
+// There are 2 cases:
+// 1) materialize m from map().
+// 2) materialize n from newin.
+// so from_map is either from current map or newin of merge_common().
+// for case 1, the control of from_map is Region, so we need to adjust it to c1.
+//
+// Regarding compile-time state, We also need to consider ABIO(in(1)) and Memory(in(2)) of from_map.
+// we reverse the iteration order in merge_common(). For the time being, we haven't merged ABIO and Memory yet.
+// I.e. we use (Ctrl, ABIO, Memory) of from_map before merging.
+//
 Node* Parse::ensure_object_materialized(Node* var, PEAState& state, SafePointNode* from_map, RegionNode* r, int pnum) {
-  assert(map() != from_map, "should be different");
-
   GraphKit kit(from_map->jvms());
-  kit.set_control(r->in(pnum));
-  EscapedState* es = state.materialize(&kit, var, from_map);
-  add_exception_states_from(kit.transfer_exceptions_into_jvms());
+  bool update_ctrl = from_map == map();
+
+  if (update_ctrl) {
+    assert(kit.control() == r, "sanity");
+    kit.set_control(r->in(pnum));
+  } else {
+    assert(kit.control() == r->in(pnum), "sanity");
+  }
+
+  EscapedState* es = state.materialize(&kit, var);
   Node* mv = es->get_materialized_value();
+  assert(kit.control() == mv->in(0), "sanity");
   r->set_req(pnum, mv->in(0));
-  do_exceptions();
+
+  if (update_ctrl) {
+    kit.set_control(r);
+  }
   return mv;
 }
 

src/hotspot/share/opto/parseHelper.cpp

@@ -456,39 +456,17 @@ static void replace_in_map(GraphKit* kit, Node* old, Node* neww) {
   }
 }
 
-EscapedState* PEAState::materialize(GraphKit* kit, Node* var, SafePointNode* map) {
+EscapedState* PEAState::materialize(GraphKit* kit, Node* var) {
   ObjID alloc = is_alias(var);
   assert(alloc != nullptr && get_object_state(alloc)->is_virtual(), "sanity check");
 #ifndef PRODUCT
   if (Verbose) {
     tty->print_cr("PEA materializes a virtual object: %d", alloc->_idx);
   }
 #endif
-
-  if (map == nullptr) {
-    JVMState* jvms = kit->sync_jvms();
-    map = kit->map();
-
-    kit->kill_dead_locals();
-    kit->clean_stack(jvms->sp());
-    jvms->set_should_reexecute(false);
-  }
-
   Compile* C = kit->C;
-  // The entire memory state is needed for slow path of the allocation
-  // since GC and deoptimization can happened.
-  Node *mem = kit->reset_memory();
-  kit->set_all_memory(mem); // Create new memory state
-
-  // clean up map/jvms before we clone a new AllocateNode.
-  AllocateNode* allocx  = new AllocateNode(C, alloc->tf(), map->control(), mem, map->i_o(),
-                                           alloc->in(AllocateNode::AllocSize),
-                                           alloc->in(AllocateNode::KlassNode),
-                                           alloc->in(AllocateNode::InitialTest));
-
   const TypeOopPtr* oop_type = var->as_Type()->type()->is_oopptr();
-  Node* objx = kit->set_output_for_allocation(allocx, oop_type);
-  allocx->jvms()->set_bci(alloc->jvms()->bci());
+  Node* objx = kit->materialize_object(alloc, oop_type);
   VirtualState* virt = static_cast<VirtualState*>(get_object_state(alloc));
 
   if (oop_type->isa_instptr()) {

src/hotspot/share/opto/partialEscape.hpp

@@ -145,7 +145,7 @@ class PEAState {
   void remove_alias(ObjID id, Node* var);
 
   void add_new_allocation(Node* obj);
-  EscapedState* materialize(GraphKit* kit, Node* var, SafePointNode* map = nullptr);
+  EscapedState* materialize(GraphKit* kit, Node* var);
 
 #ifndef PRODUCT
   void print_on(outputStream* os) const;