8190817: deopt special-case for _return_register_finalizer is confusing and leads to bugs
Reviewed-by: vlivanov, dpochepk
--- a/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -447,7 +447,8 @@
}
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 @@
__ bind(L);
}
- __ dispatch_next(state, step);
+ if (continuation == NULL) {
+ __ dispatch_next(state, step);
+ } else {
+ __ jump_to_entry(continuation);
+ }
return entry;
}
--- a/src/hotspot/cpu/aarch64/templateTable_aarch64.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/aarch64/templateTable_aarch64.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -2195,13 +2195,6 @@
__ 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.
--- a/src/hotspot/cpu/arm/templateInterpreterGenerator_arm.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/arm/templateInterpreterGenerator_arm.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -314,7 +314,7 @@
}
-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 @@
__ bind(L);
}
- __ dispatch_next(state, step);
+ if (continuation == NULL) {
+ __ dispatch_next(state, step);
+ } else {
+ __ jump_to_entry(continuation);
+ }
return entry;
}
--- a/src/hotspot/cpu/arm/templateTable_arm.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/arm/templateTable_arm.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -2844,19 +2844,6 @@
__ 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.
--- a/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -694,7 +694,7 @@
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 @@
__ 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;
}
--- a/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -687,7 +687,8 @@
}
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 @@
__ 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");
--- a/src/hotspot/cpu/sparc/templateInterpreterGenerator_sparc.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/sparc/templateInterpreterGenerator_sparc.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -313,7 +313,7 @@
}
-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 @@
__ should_not_reach_here();
__ bind(L);
}
- __ dispatch_next(state, step);
+ if (continuation == NULL) {
+ __ dispatch_next(state, step);
+ } else {
+ __ jump_to_entry(continuation);
+ }
return entry;
}
--- a/src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -237,7 +237,7 @@
}
-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 @@
__ should_not_reach_here();
__ bind(L);
}
- __ dispatch_next(state, step);
+ if (continuation == NULL) {
+ __ dispatch_next(state, step);
+ } else {
+ __ jump_to_entry(continuation);
+ }
return entry;
}
--- a/src/hotspot/cpu/x86/templateTable_x86.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/cpu/x86/templateTable_x86.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -2563,13 +2563,6 @@
__ 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;
--- a/src/hotspot/share/interpreter/templateInterpreter.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/share/interpreter/templateInterpreter.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -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::_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 @@
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 @@
// 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);
- if (code == Bytecodes::_return) {
+ Bytecodes::Code code = Bytecodes::code_at(method, bcp);
+ 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);
}
--- a/src/hotspot/share/interpreter/templateInterpreter.hpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/share/interpreter/templateInterpreter.hpp Tue Nov 21 09:04:42 2017 -0800
@@ -92,8 +92,10 @@
public:
enum MoreConstants {
- number_of_return_entries = number_of_states, // number of return entry points
- number_of_deopt_entries = number_of_states, // number of deoptimization entry points
+ max_invoke_length = 5, // invokedynamic is the longest
+ 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 @@
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 @@
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
--- a/src/hotspot/share/interpreter/templateInterpreterGenerator.cpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/share/interpreter/templateInterpreterGenerator.cpp Tue Nov 21 09:04:42 2017 -0800
@@ -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 @@
{ 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),
- generate_return_entry_for(itos, i, index_size),
- generate_return_entry_for(itos, i, index_size),
- generate_return_entry_for(itos, i, index_size),
+ return_itos,
+ return_itos,
+ return_itos,
+ 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 @@
);
}
- { 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 @@
// 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);
+ }
+
}
//------------------------------------------------------------------------------------------------------------------------
--- a/src/hotspot/share/interpreter/templateInterpreterGenerator.hpp Fri Nov 17 20:56:14 2017 +0300
+++ b/src/hotspot/share/interpreter/templateInterpreterGenerator.hpp Tue Nov 21 09:04:42 2017 -0800
@@ -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 @@
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();
--- a/test/hotspot/jtreg/compiler/runtime/Test8168712.java Fri Nov 17 20:56:14 2017 +0300
+++ b/test/hotspot/jtreg/compiler/runtime/Test8168712.java Tue Nov 21 09:04:42 2017 -0800
@@ -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