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

Reviewed-by: vlivanov, dpochepk
This commit is contained in:
Dean Long 2017-11-21 09:04:42 -08:00
parent cdabc7a61c
commit fb89c1729c
14 changed files with 92 additions and 75 deletions

View File

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

View File

@ -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.

View File

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

View File

@ -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.

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -92,8 +92,10 @@ class TemplateInterpreter: public AbstractInterpreter {
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 @@ 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

View File

@ -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),
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 @@ 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);
}
}
//------------------------------------------------------------------------------------------------------------------------

View File

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

View File

@ -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