8261158: JVMState should not be shared between SafePointNodes

Reviewed-by: vlivanov, kvn

src/hotspot/share/adlc/archDesc.cpp

@@ -147,7 +147,7 @@ ArchDesc::ArchDesc()
     _internalMatch(cmpstr,hashstr, Form::arena),
     _chainRules(cmpstr,hashstr, Form::arena),
     _cisc_spill_operand(NULL),
-    _needs_clone_jvms(false) {
+    _needs_deep_clone_jvms(false) {
 
       // Initialize the opcode to MatchList table with NULLs
       for( int i=0; i<_last_opcode; ++i ) {

src/hotspot/share/adlc/archDesc.hpp

@@ -123,9 +123,9 @@ private:
 
   // If a Call node uses $constanttablebase, it gets MachConstantBaseNode
   // by the matcher and the matcher will modify the jvms. If so, jvm states
-  // always have to be cloned when a node is cloned.  Adlc generates
-  // Compile::needs_clone_jvms() accordingly.
-  bool _needs_clone_jvms;
+  // always have to be deep cloned when a node is cloned. Adlc generates
+  // Compile::needs_deep_clone_jvms() accordingly.
+  bool _needs_deep_clone_jvms;
 
   // Methods for outputting the DFA
   void gen_match(FILE *fp, MatchList &mlist, ProductionState &status, Dict &operands_chained_from);
@@ -295,7 +295,7 @@ public:
   void addPreHeaderBlocks(FILE *fp_hpp);
   void addHeaderBlocks(FILE *fp_hpp);
   void addSourceBlocks(FILE *fp_cpp);
-  void generate_needs_clone_jvms(FILE *fp_cpp);
+  void generate_needs_deep_clone_jvms(FILE *fp_cpp);
   void generate_adlc_verification(FILE *fp_cpp);
 
   // output declaration of class State

src/hotspot/share/adlc/main.cpp

@@ -305,7 +305,7 @@ int main(int argc, char *argv[])
   AD.buildInstructMatchCheck(AD._CPP_file._fp);  // .cpp
   // define methods for machine dependent frame management
   AD.buildFrameMethods(AD._CPP_file._fp);         // .cpp
-  AD.generate_needs_clone_jvms(AD._CPP_file._fp);
+  AD.generate_needs_deep_clone_jvms(AD._CPP_file._fp);
 
   // do this last:
   AD.addPreprocessorChecks(AD._CPP_file._fp);     // .cpp

src/hotspot/share/adlc/output_c.cpp

@@ -1792,7 +1792,7 @@ void ArchDesc::defineExpand(FILE *fp, InstructForm *node) {
     if (node->is_ideal_call() != Form::invalid_type &&
         node->is_ideal_call() != Form::JAVA_LEAF) {
       fprintf(fp, "  // MachConstantBaseNode added in matcher.\n");
-      _needs_clone_jvms = true;
+      _needs_deep_clone_jvms = true;
     } else {
       fprintf(fp, "  add_req(C->mach_constant_base_node());\n");
     }
@@ -3603,9 +3603,9 @@ char reg_save_policy(const char *calling_convention) {
   return callconv;
 }
 
-void ArchDesc::generate_needs_clone_jvms(FILE *fp_cpp) {
-  fprintf(fp_cpp, "bool Compile::needs_clone_jvms() { return %s; }\n\n",
-          _needs_clone_jvms ? "true" : "false");
+void ArchDesc::generate_needs_deep_clone_jvms(FILE *fp_cpp) {
+  fprintf(fp_cpp, "bool Compile::needs_deep_clone_jvms() { return %s; }\n\n",
+          _needs_deep_clone_jvms ? "true" : "false");
 }
 
 //---------------------------generate_assertion_checks-------------------

src/hotspot/share/opto/callnode.cpp

@@ -1362,6 +1362,7 @@ SafePointNode* SafePointNode::next_exception() const {
 //------------------------------Ideal------------------------------------------
 // Skip over any collapsed Regions
 Node *SafePointNode::Ideal(PhaseGVN *phase, bool can_reshape) {
+  assert(_jvms == NULL || ((uintptr_t)_jvms->map() & 1) || _jvms->map() == this, "inconsistent JVMState");
   return remove_dead_region(phase, can_reshape) ? this : NULL;
 }
 

src/hotspot/share/opto/callnode.hpp

@@ -352,6 +352,18 @@ public:
   }
 
   JVMState* jvms() const { return _jvms; }
+  virtual bool needs_deep_clone_jvms(Compile* C) { return false; }
+  void clone_jvms(Compile* C) {
+    if (jvms() != NULL) {
+      if (needs_deep_clone_jvms(C)) {
+        set_jvms(jvms()->clone_deep(C));
+        jvms()->set_map_deep(this);
+      } else {
+        jvms()->clone_shallow(C)->bind_map(this);
+      }
+    }
+  }
+
  private:
   void verify_input(JVMState* jvms, uint idx) const {
     assert(verify_jvms(jvms), "jvms must match");
@@ -615,14 +627,8 @@ public:
   virtual bool        guaranteed_safepoint()  { return true; }
   // For macro nodes, the JVMState gets modified during expansion. If calls
   // use MachConstantBase, it gets modified during matching. So when cloning
-  // the node the JVMState must be cloned. Default is not to clone.
-  virtual bool needs_clone_jvms(Compile* C) { return C->needs_clone_jvms(); }
-  void clone_jvms(Compile* C) {
-    if ((jvms() != NULL) && needs_clone_jvms(C)) {
-      set_jvms(jvms()->clone_deep(C));
-      jvms()->set_map_deep(this);
-    }
-  }
+  // the node the JVMState must be deep cloned. Default is to shallow clone.
+  virtual bool needs_deep_clone_jvms(Compile* C) { return C->needs_deep_clone_jvms(); }
 
   // Returns true if the call may modify n
   virtual bool        may_modify(const TypeOopPtr* t_oop, PhaseTransform* phase);
@@ -736,10 +742,10 @@ public:
   bool is_boxing_method() const {
     return is_macro() && (method() != NULL) && method()->is_boxing_method();
   }
-  // Late inlining modifies the JVMState, so we need to clone it
+  // Late inlining modifies the JVMState, so we need to deep clone it
   // when the call node is cloned (because it is macro node).
-  virtual bool needs_clone_jvms(Compile* C) {
-    return is_boxing_method() || CallNode::needs_clone_jvms(C);
+  virtual bool needs_deep_clone_jvms(Compile* C) {
+    return is_boxing_method() || CallNode::needs_deep_clone_jvms(C);
   }
 
   virtual int         Opcode() const;
@@ -762,10 +768,10 @@ public:
     init_class_id(Class_CallDynamicJava);
   }
 
-  // Late inlining modifies the JVMState, so we need to clone it
+  // Late inlining modifies the JVMState, so we need to deep clone it
   // when the call node is cloned.
-  virtual bool needs_clone_jvms(Compile* C) {
-    return IncrementalInlineVirtual || CallNode::needs_clone_jvms(C);
+  virtual bool needs_deep_clone_jvms(Compile* C) {
+    return IncrementalInlineVirtual || CallNode::needs_deep_clone_jvms(C);
   }
 
   int _vtable_index;
@@ -917,8 +923,8 @@ public:
   virtual uint size_of() const; // Size is bigger
   AllocateNode(Compile* C, const TypeFunc *atype, Node *ctrl, Node *mem, Node *abio,
                Node *size, Node *klass_node, Node *initial_test);
-  // Expansion modifies the JVMState, so we need to clone it
-  virtual bool needs_clone_jvms(Compile* C) { return true; }
+  // Expansion modifies the JVMState, so we need to deep clone it
+  virtual bool needs_deep_clone_jvms(Compile* C) { return true; }
   virtual int Opcode() const;
   virtual uint ideal_reg() const { return Op_RegP; }
   virtual bool        guaranteed_safepoint()  { return false; }
@@ -1131,8 +1137,8 @@ public:
   virtual bool        guaranteed_safepoint()  { return false; }
 
   virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
-  // Expansion modifies the JVMState, so we need to clone it
-  virtual bool needs_clone_jvms(Compile* C) { return true; }
+  // Expansion modifies the JVMState, so we need to deep clone it
+  virtual bool needs_deep_clone_jvms(Compile* C) { return true; }
 
   bool is_nested_lock_region(); // Is this Lock nested?
   bool is_nested_lock_region(Compile * c); // Why isn't this Lock nested?

src/hotspot/share/opto/compile.hpp

@@ -792,7 +792,7 @@ class Compile : public Phase {
   MachConstantBaseNode*     mach_constant_base_node();
   bool                  has_mach_constant_base_node() const { return _mach_constant_base_node != NULL; }
   // Generated by adlc, true if CallNode requires MachConstantBase.
-  bool                      needs_clone_jvms();
+  bool                      needs_deep_clone_jvms();
 
   // Handy undefined Node
   Node*             top() const                 { return _top; }

src/hotspot/share/opto/node.cpp

@@ -558,8 +558,6 @@ Node *Node::clone() const {
     }
   }
   if (n->is_Call()) {
-    // cloning CallNode may need to clone JVMState
-    n->as_Call()->clone_jvms(C);
     // CallGenerator is linked to the original node.
     CallGenerator* cg = n->as_Call()->generator();
     if (cg != NULL) {
@@ -572,6 +570,9 @@ Node *Node::clone() const {
     }
   }
   if (n->is_SafePoint()) {
+    // Scalar replacement and macro expansion might modify the JVMState.
+    // Clone it to make sure it's not shared between SafePointNodes.
+    n->as_SafePoint()->clone_jvms(C);
     n->as_SafePoint()->clone_replaced_nodes();
   }
   return n;                     // Return the clone