8190817: deopt special-case for _return_register_finalizer is confusing and leads to bugs

Reviewed-by: vlivanov, dpochepk

src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp

@@ -447,7 +447,8 @@ address TemplateInterpreterGenerator::generate_return_entry_for(TosState state,
 }
 
 address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state,
-                                                               int step) {
+                                                               int step,
+                                                               address continuation) {
   address entry = __ pc();
   __ restore_bcp();
   __ restore_locals();
@@ -505,7 +506,11 @@ address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state,
     __ bind(L);
   }
 
-  __ dispatch_next(state, step);
+  if (continuation == NULL) {
+    __ dispatch_next(state, step);
+  } else {
+    __ jump_to_entry(continuation);
+  }
   return entry;
 }
 

src/hotspot/cpu/aarch64/templateTable_aarch64.cpp

@@ -2195,13 +2195,6 @@ void TemplateTable::_return(TosState state)
     __ bind(skip_register_finalizer);
   }
 
-  // Explicitly reset last_sp, for handling special case in TemplateInterpreter::deopt_reexecute_entry
-#ifdef ASSERT
-  if (state == vtos) {
-    __ str(zr, Address(rfp, frame::interpreter_frame_last_sp_offset * wordSize));
-  }
-#endif
-
   // Issue a StoreStore barrier after all stores but before return
   // from any constructor for any class with a final field.  We don't
   // know if this is a finalizer, so we always do so.

src/hotspot/cpu/arm/templateInterpreterGenerator_arm.cpp

@@ -314,7 +314,7 @@ address TemplateInterpreterGenerator::generate_return_entry_for(TosState state,
 }
 
 
-address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, int step) {
+address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, int step, address continuation) {
   address entry = __ pc();
 
   __ interp_verify_oop(R0_tos, state, __FILE__, __LINE__);
@@ -343,7 +343,11 @@ address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, i
     __ bind(L);
   }
 
-  __ dispatch_next(state, step);
+  if (continuation == NULL) {
+    __ dispatch_next(state, step);
+  } else {
+    __ jump_to_entry(continuation);
+  }
 
   return entry;
 }

src/hotspot/cpu/arm/templateTable_arm.cpp

@@ -2844,19 +2844,6 @@ void TemplateTable::_return(TosState state) {
     __ bind(skip_register_finalizer);
   }
 
-  // Explicitly reset last_sp, for handling special case in TemplateInterpreter::deopt_reexecute_entry
-#ifdef ASSERT
-  if (state == vtos) {
-#ifndef AARCH64
-    __ mov(Rtemp, 0);
-    __ str(Rtemp, Address(FP, frame::interpreter_frame_last_sp_offset * wordSize));
-#else
-    __ restore_sp_after_call(Rtemp);
-    __ restore_stack_top();
-#endif
-  }
-#endif
-
   // Narrow result if state is itos but result type is smaller.
   // Need to narrow in the return bytecode rather than in generate_return_entry
   // since compiled code callers expect the result to already be narrowed.

src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp

@@ -694,7 +694,7 @@ address TemplateInterpreterGenerator::generate_return_entry_for(TosState state,
   return entry;
 }
 
-address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, int step) {
+address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, int step, address continuation) {
   address entry = __ pc();
   // If state != vtos, we're returning from a native method, which put it's result
   // into the result register. So move the value out of the return register back
@@ -721,7 +721,11 @@ address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, i
   __ check_and_forward_exception(R11_scratch1, R12_scratch2);
 
   // Start executing bytecodes.
-  __ dispatch_next(state, step);
+  if (continuation == NULL) {
+    __ dispatch_next(state, step);
+  } else {
+    __ jump_to_entry(continuation, R11_scratch1);
+  }
 
   return entry;
 }

src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp

@@ -687,7 +687,8 @@ address TemplateInterpreterGenerator::generate_return_entry_for (TosState state,
 }
 
 address TemplateInterpreterGenerator::generate_deopt_entry_for (TosState state,
-                                                               int step) {
+                                                               int step,
+                                                               address continuation) {
   address entry = __ pc();
 
   BLOCK_COMMENT("deopt_entry {");
@@ -710,7 +711,11 @@ address TemplateInterpreterGenerator::generate_deopt_entry_for (TosState state,
     __ should_not_reach_here();
     __ bind(L);
   }
-  __ dispatch_next(state, step);
+  if (continuation == NULL) {
+    __ dispatch_next(state, step);
+  } else {
+    __ jump_to_entry(continuation, Z_R1_scratch);
+  }
 
   BLOCK_COMMENT("} deopt_entry");
 

src/hotspot/cpu/sparc/templateInterpreterGenerator_sparc.cpp

@@ -313,7 +313,7 @@ address TemplateInterpreterGenerator::generate_return_entry_for(TosState state,
 }
 
 
-address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, int step) {
+address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, int step, address continuation) {
   address entry = __ pc();
   __ get_constant_pool_cache(LcpoolCache); // load LcpoolCache
 #if INCLUDE_JVMCI
@@ -350,7 +350,11 @@ address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, i
     __ should_not_reach_here();
     __ bind(L);
   }
-  __ dispatch_next(state, step);
+  if (continuation == NULL) {
+    __ dispatch_next(state, step);
+  } else {
+    __ jump_to_entry(continuation);
+  }
   return entry;
 }
 

src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp

@@ -237,7 +237,7 @@ address TemplateInterpreterGenerator::generate_return_entry_for(TosState state,
 }
 
 
-address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, int step) {
+address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, int step, address continuation) {
   address entry = __ pc();
 
 #ifndef _LP64
@@ -291,7 +291,11 @@ address TemplateInterpreterGenerator::generate_deopt_entry_for(TosState state, i
     __ should_not_reach_here();
     __ bind(L);
   }
-  __ dispatch_next(state, step);
+  if (continuation == NULL) {
+    __ dispatch_next(state, step);
+  } else {
+    __ jump_to_entry(continuation);
+  }
   return entry;
 }
 

src/hotspot/cpu/x86/templateTable_x86.cpp

@@ -2563,13 +2563,6 @@ void TemplateTable::_return(TosState state) {
     __ bind(skip_register_finalizer);
   }
 
-  // Explicitly reset last_sp, for handling special case in TemplateInterpreter::deopt_reexecute_entry
-#ifdef ASSERT
-  if (state == vtos) {
-    __ movptr(Address(rbp, frame::interpreter_frame_last_sp_offset * wordSize), (int32_t)NULL_WORD);
-  }
-#endif
-
 #ifdef _LP64
   if (SafepointMechanism::uses_thread_local_poll() && _desc->bytecode() != Bytecodes::_return_register_finalizer) {
     Label no_safepoint;

src/hotspot/share/interpreter/templateInterpreter.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -195,6 +195,7 @@ EntryPoint TemplateInterpreter::_trace_code;
 EntryPoint TemplateInterpreter::_return_entry[TemplateInterpreter::number_of_return_entries];
 EntryPoint TemplateInterpreter::_earlyret_entry;
 EntryPoint TemplateInterpreter::_deopt_entry [TemplateInterpreter::number_of_deopt_entries ];
+address    TemplateInterpreter::_deopt_reexecute_return_entry;
 EntryPoint TemplateInterpreter::_safept_entry;
 
 address TemplateInterpreter::_invoke_return_entry[TemplateInterpreter::number_of_return_addrs];
@@ -248,14 +249,18 @@ address TemplateInterpreter::return_entry(TosState state, int length, Bytecodes:
     return _invokedynamic_return_entry[index];
   default:
     assert(!Bytecodes::is_invoke(code), "invoke instructions should be handled separately: %s", Bytecodes::name(code));
-    return _return_entry[length].entry(state);
+    address entry = _return_entry[length].entry(state);
+    vmassert(entry != NULL, "unsupported return entry requested, length=%d state=%d", length, index);
+    return entry;
   }
 }
 
 
 address TemplateInterpreter::deopt_entry(TosState state, int length) {
   guarantee(0 <= length && length < Interpreter::number_of_deopt_entries, "illegal length");
-  return _deopt_entry[length].entry(state);
+  address entry = _deopt_entry[length].entry(state);
+  vmassert(entry != NULL, "unsupported deopt entry requested, length=%d state=%d", length, TosState_as_index(state));
+  return entry;
 }
 
 //------------------------------------------------------------------------------------------------------------------------
@@ -313,14 +318,14 @@ address TemplateInterpreter::deopt_continue_after_entry(Method* method, address
 //       that do not return "Interpreter::deopt_entry(vtos, 0)"
 address TemplateInterpreter::deopt_reexecute_entry(Method* method, address bcp) {
   assert(method->contains(bcp), "just checkin'");
-  Bytecodes::Code code   = Bytecodes::java_code_at(method, bcp);
+  Bytecodes::Code code   = Bytecodes::code_at(method, bcp);
-  if (code == Bytecodes::_return) {
+  if (code == Bytecodes::_return_register_finalizer) {
     // This is used for deopt during registration of finalizers
     // during Object.<init>.  We simply need to resume execution at
     // the standard return vtos bytecode to pop the frame normally.
     // reexecuting the real bytecode would cause double registration
     // of the finalizable object.
-    return _normal_table.entry(Bytecodes::_return).entry(vtos);
+    return Interpreter::deopt_reexecute_return_entry();
   } else {
     return AbstractInterpreter::deopt_reexecute_entry(method, bcp);
   }

src/hotspot/share/interpreter/templateInterpreter.hpp

@@ -92,8 +92,10 @@ class TemplateInterpreter: public AbstractInterpreter {
  public:
 
   enum MoreConstants {
-    number_of_return_entries  = number_of_states,               // number of return entry points
+    max_invoke_length = 5,    // invokedynamic is the longest
-    number_of_deopt_entries   = number_of_states,               // number of deoptimization entry points
+    max_bytecode_length = 6,  // worse case is wide iinc, "reexecute" bytecodes are excluded because "skip" will be 0
+    number_of_return_entries  = max_invoke_length + 1,          // number of return entry points
+    number_of_deopt_entries   = max_bytecode_length + 1,        // number of deoptimization entry points
     number_of_return_addrs    = number_of_states                // number of return addresses
   };
 
@@ -119,6 +121,7 @@ class TemplateInterpreter: public AbstractInterpreter {
   static EntryPoint _return_entry[number_of_return_entries];    // entry points to return to from a call
   static EntryPoint _earlyret_entry;                            // entry point to return early from a call
   static EntryPoint _deopt_entry[number_of_deopt_entries];      // entry points to return to from a deoptimization
+  static address    _deopt_reexecute_return_entry;
   static EntryPoint _safept_entry;
 
   static address _invoke_return_entry[number_of_return_addrs];           // for invokestatic, invokespecial, invokevirtual return entries
@@ -173,6 +176,7 @@ class TemplateInterpreter: public AbstractInterpreter {
   static address* invoke_return_entry_table_for(Bytecodes::Code code);
 
   static address deopt_entry(TosState state, int length);
+  static address deopt_reexecute_return_entry()                 { return _deopt_reexecute_return_entry; }
   static address return_entry(TosState state, int length, Bytecodes::Code code);
 
   // Safepoint support

src/hotspot/share/interpreter/templateInterpreterGenerator.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -84,15 +84,17 @@ void TemplateInterpreterGenerator::generate_all() {
 
   { CodeletMark cm(_masm, "return entry points");
     const int index_size = sizeof(u2);
-    for (int i = 0; i < Interpreter::number_of_return_entries; i++) {
+    Interpreter::_return_entry[0] = EntryPoint();
+    for (int i = 1; i < Interpreter::number_of_return_entries; i++) {
+      address return_itos = generate_return_entry_for(itos, i, index_size);
       Interpreter::_return_entry[i] =
         EntryPoint(
-                   generate_return_entry_for(itos, i, index_size),
+                   return_itos,
-                   generate_return_entry_for(itos, i, index_size),
+                   return_itos,
-                   generate_return_entry_for(itos, i, index_size),
+                   return_itos,
-                   generate_return_entry_for(itos, i, index_size),
+                   return_itos,
                    generate_return_entry_for(atos, i, index_size),
-                   generate_return_entry_for(itos, i, index_size),
+                   return_itos,
                    generate_return_entry_for(ltos, i, index_size),
                    generate_return_entry_for(ftos, i, index_size),
                    generate_return_entry_for(dtos, i, index_size),
@@ -134,24 +136,6 @@ void TemplateInterpreterGenerator::generate_all() {
                  );
   }
 
-  { CodeletMark cm(_masm, "deoptimization entry points");
-    for (int i = 0; i < Interpreter::number_of_deopt_entries; i++) {
-      Interpreter::_deopt_entry[i] =
-        EntryPoint(
-                   generate_deopt_entry_for(itos, i),
-                   generate_deopt_entry_for(itos, i),
-                   generate_deopt_entry_for(itos, i),
-                   generate_deopt_entry_for(itos, i),
-                   generate_deopt_entry_for(atos, i),
-                   generate_deopt_entry_for(itos, i),
-                   generate_deopt_entry_for(ltos, i),
-                   generate_deopt_entry_for(ftos, i),
-                   generate_deopt_entry_for(dtos, i),
-                   generate_deopt_entry_for(vtos, i)
-                   );
-    }
-  }
-
   { CodeletMark cm(_masm, "result handlers for native calls");
     // The various result converter stublets.
     int is_generated[Interpreter::number_of_result_handlers];
@@ -250,6 +234,31 @@ void TemplateInterpreterGenerator::generate_all() {
   // installation of code in other places in the runtime
   // (ExcutableCodeManager calls not needed to copy the entries)
   set_safepoints_for_all_bytes();
+
+  { CodeletMark cm(_masm, "deoptimization entry points");
+    Interpreter::_deopt_entry[0] = EntryPoint();
+    Interpreter::_deopt_entry[0].set_entry(vtos, generate_deopt_entry_for(vtos, 0));
+    for (int i = 1; i < Interpreter::number_of_deopt_entries; i++) {
+      address deopt_itos = generate_deopt_entry_for(itos, i);
+      Interpreter::_deopt_entry[i] =
+        EntryPoint(
+                   deopt_itos, /* btos */
+                   deopt_itos, /* ztos */
+                   deopt_itos, /* ctos */
+                   deopt_itos, /* stos */
+                   generate_deopt_entry_for(atos, i),
+                   deopt_itos, /* itos */
+                   generate_deopt_entry_for(ltos, i),
+                   generate_deopt_entry_for(ftos, i),
+                   generate_deopt_entry_for(dtos, i),
+                   generate_deopt_entry_for(vtos, i)
+                   );
+    }
+    address return_continuation = Interpreter::_normal_table.entry(Bytecodes::_return).entry(vtos);
+    vmassert(return_continuation != NULL, "return entry not generated yet");
+    Interpreter::_deopt_reexecute_return_entry = generate_deopt_entry_for(vtos, 0, return_continuation);
+  }
+
 }
 
 //------------------------------------------------------------------------------------------------------------------------

src/hotspot/share/interpreter/templateInterpreterGenerator.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -54,7 +54,7 @@ class TemplateInterpreterGenerator: public AbstractInterpreterGenerator {
   address generate_ArrayIndexOutOfBounds_handler(const char* name);
   address generate_return_entry_for(TosState state, int step, size_t index_size);
   address generate_earlyret_entry_for(TosState state);
-  address generate_deopt_entry_for(TosState state, int step);
+  address generate_deopt_entry_for(TosState state, int step, address continuation = NULL);
   address generate_safept_entry_for(TosState state, address runtime_entry);
   void    generate_throw_exception();
 

test/hotspot/jtreg/compiler/runtime/Test8168712.java

@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @requires vm.simpleArch == "x64" & vm.debug
+ * @requires vm.debug
  * @bug 8168712
  *
  * @run main/othervm -XX:CompileCommand=compileonly,Test8168712.* -XX:CompileCommand=compileonly,*Object.* -XX:+DTraceMethodProbes -XX:-UseOnStackReplacement -XX:+DeoptimizeRandom compiler.runtime.Test8168712