diff --git a/runtime/vm/deopt_instructions.cc b/runtime/vm/deopt_instructions.cc index 31f08cd3924..345db49623f 100644 --- a/runtime/vm/deopt_instructions.cc +++ b/runtime/vm/deopt_instructions.cc @@ -1077,7 +1077,13 @@ void DeoptInfoBuilder::AddCopy(Value* value, deopt_instr = new (zone()) DeoptMaterializedObjectRefInstr(index); } else { ASSERT(!source_loc.IsInvalid()); - switch (value->definition()->representation()) { +#if defined(TARGET_ARCH_DBC) + Representation rep = + (value == NULL) ? kTagged : value->definition()->representation(); +#else + Representation rep = value->definition()->representation(); +#endif + switch (rep) { case kTagged: deopt_instr = new (zone()) DeoptWordInstr(ToCpuRegisterSource(source_loc)); diff --git a/runtime/vm/flow_graph_compiler.cc b/runtime/vm/flow_graph_compiler.cc index a3db43828ea..d0a2cb111a7 100644 --- a/runtime/vm/flow_graph_compiler.cc +++ b/runtime/vm/flow_graph_compiler.cc @@ -791,7 +791,7 @@ void FlowGraphCompiler::AddStubCallTarget(const Code& code) { } -void FlowGraphCompiler::AddDeoptIndexAtCall(intptr_t deopt_id) { +CompilerDeoptInfo* FlowGraphCompiler::AddDeoptIndexAtCall(intptr_t deopt_id) { ASSERT(is_optimizing()); ASSERT(!intrinsic_mode()); CompilerDeoptInfo* info = @@ -800,6 +800,7 @@ void FlowGraphCompiler::AddDeoptIndexAtCall(intptr_t deopt_id) { pending_deoptimization_env_); info->set_pc_offset(assembler()->CodeSize()); deopt_infos_.Add(info); + return info; } diff --git a/runtime/vm/flow_graph_compiler.h b/runtime/vm/flow_graph_compiler.h index e8b013fdc58..8d8606b8704 100644 --- a/runtime/vm/flow_graph_compiler.h +++ b/runtime/vm/flow_graph_compiler.h @@ -122,6 +122,9 @@ class CompilerDeoptInfo : public ZoneAllocated { deopt_id_(deopt_id), reason_(reason), flags_(flags), +#if defined(TARGET_ARCH_DBC) + lazy_deopt_with_result_(false), +#endif deopt_env_(deopt_env) { ASSERT(deopt_env != NULL); } @@ -143,6 +146,18 @@ class CompilerDeoptInfo : public ZoneAllocated { uint32_t flags() const { return flags_; } const Environment* deopt_env() const { return deopt_env_; } +#if defined(TARGET_ARCH_DBC) + // On DBC calls return results on the stack but not all calls have a result. + // This needs to be taken into account when constructing lazy deoptimization + // environment. + // For calls with results we add a deopt instruction that would copy top + // of the stack from optimized frame to unoptimized frame effectively + // preserving the result of the call. + // For calls with no results we don't emit such instruction - because there + // is no result pushed by the return sequence. + void mark_lazy_deopt_with_result() { lazy_deopt_with_result_ = true; } +#endif + private: void EmitMaterializations(Environment* env, DeoptInfoBuilder* builder); @@ -153,6 +168,9 @@ class CompilerDeoptInfo : public ZoneAllocated { const intptr_t deopt_id_; const ICData::DeoptReasonId reason_; const uint32_t flags_; +#if defined(TARGET_ARCH_DBC) + bool lazy_deopt_with_result_; +#endif Environment* deopt_env_; DISALLOW_COPY_AND_ASSIGN(CompilerDeoptInfo); @@ -499,7 +517,7 @@ class FlowGraphCompiler : public ValueObject { uint16_t ToEmbeddableCid(intptr_t cid, Instruction* instruction); #endif // defined(TARGET_ARCH_DBC) - void AddDeoptIndexAtCall(intptr_t deopt_id); + CompilerDeoptInfo* AddDeoptIndexAtCall(intptr_t deopt_id); void AddSlowPathCode(SlowPathCode* slow_path); @@ -583,11 +601,16 @@ class FlowGraphCompiler : public ValueObject { bool EndCodeSourceRange(TokenPosition token_pos); #if defined(TARGET_ARCH_DBC) + enum CallResult { + kHasResult, + kNoResult, + }; void RecordAfterCallHelper(TokenPosition token_pos, intptr_t deopt_id, intptr_t argument_count, + CallResult result, LocationSummary* locs); - void RecordAfterCall(Instruction* instr); + void RecordAfterCall(Instruction* instr, CallResult result); #endif private: diff --git a/runtime/vm/flow_graph_compiler_dbc.cc b/runtime/vm/flow_graph_compiler_dbc.cc index 745fcf98e28..eb3660a4f7d 100644 --- a/runtime/vm/flow_graph_compiler_dbc.cc +++ b/runtime/vm/flow_graph_compiler_dbc.cc @@ -118,6 +118,11 @@ RawTypedData* CompilerDeoptInfo::CreateDeoptInfo(FlowGraphCompiler* compiler, // will be able to find them during materialization. slot_ix = builder->EmitMaterializationArguments(slot_ix); + if (lazy_deopt_with_result_) { + ASSERT(reason() == ICData::kDeoptAtCall); + builder->AddCopy(NULL, Location::StackSlot(stack_height), slot_ix++); + } + // For the innermost environment, set outgoing arguments and the locals. for (intptr_t i = current->Length() - 1; i >= current->fixed_parameter_count(); i--) { @@ -179,6 +184,7 @@ RawTypedData* CompilerDeoptInfo::CreateDeoptInfo(FlowGraphCompiler* compiler, void FlowGraphCompiler::RecordAfterCallHelper(TokenPosition token_pos, intptr_t deopt_id, intptr_t argument_count, + CallResult result, LocationSummary* locs) { RecordSafepoint(locs); // Marks either the continuation point in unoptimized code or the @@ -190,7 +196,10 @@ void FlowGraphCompiler::RecordAfterCallHelper(TokenPosition token_pos, // On all other architectures caller drops outgoing arguments itself // hence the difference. pending_deoptimization_env_->DropArguments(argument_count); - AddDeoptIndexAtCall(deopt_id_after); + CompilerDeoptInfo* info = AddDeoptIndexAtCall(deopt_id_after); + if (result == kHasResult) { + info->mark_lazy_deopt_with_result(); + } // This descriptor is needed for exception handling in optimized code. AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id_after, token_pos); } else { @@ -201,9 +210,9 @@ void FlowGraphCompiler::RecordAfterCallHelper(TokenPosition token_pos, } -void FlowGraphCompiler::RecordAfterCall(Instruction* instr) { +void FlowGraphCompiler::RecordAfterCall(Instruction* instr, CallResult result) { RecordAfterCallHelper(instr->token_pos(), instr->deopt_id(), - instr->ArgumentCount(), instr->locs()); + instr->ArgumentCount(), result, instr->locs()); } @@ -263,7 +272,9 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos, locs->SetStackBit(locs->out(0).reg()); } AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id, token_pos); - RecordAfterCallHelper(token_pos, deopt_id, 0, locs); + const intptr_t kArgCount = 0; + RecordAfterCallHelper(token_pos, deopt_id, kArgCount, + FlowGraphCompiler::kHasResult, locs); if (is_optimizing()) { // Assert assignable keeps the instance on the stack as the result, // all other arguments are popped. diff --git a/runtime/vm/intermediate_language.cc b/runtime/vm/intermediate_language.cc index 373aa483729..78675d997f1 100644 --- a/runtime/vm/intermediate_language.cc +++ b/runtime/vm/intermediate_language.cc @@ -3125,7 +3125,7 @@ void InstanceCallInstr::EmitNativeCode(FlowGraphCompiler* compiler) { } compiler->AddCurrentDescriptor(RawPcDescriptors::kIcCall, deopt_id(), token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); if (compiler->is_optimizing()) { __ PopLocal(locs()->out(0).reg()); @@ -3273,7 +3273,7 @@ void StaticCallInstr::EmitNativeCode(FlowGraphCompiler* compiler) { __ StaticCall(ArgumentCount(), argdesc_kidx); compiler->AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id(), token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); __ PopLocal(locs()->out(0).reg()); } else { const intptr_t ic_data_kidx = __ AddConstant(*call_ic_data); @@ -3281,7 +3281,7 @@ void StaticCallInstr::EmitNativeCode(FlowGraphCompiler* compiler) { __ IndirectStaticCall(ArgumentCount(), argdesc_kidx); compiler->AddCurrentDescriptor(RawPcDescriptors::kUnoptStaticCall, deopt_id(), token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); } #endif // !defined(TARGET_ARCH_DBC) } diff --git a/runtime/vm/intermediate_language_dbc.cc b/runtime/vm/intermediate_language_dbc.cc index efe48b14acc..60e9e940843 100644 --- a/runtime/vm/intermediate_language_dbc.cc +++ b/runtime/vm/intermediate_language_dbc.cc @@ -204,7 +204,7 @@ EMIT_NATIVE_CODE(InstanceOf, __ InstanceOf(negate_result() ? 1 : 0); compiler->AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id(), token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); if (compiler->is_optimizing()) { __ PopLocal(locs()->out(0).reg()); } @@ -227,7 +227,7 @@ EMIT_NATIVE_CODE(AssertBoolean, __ AssertBoolean(Isolate::Current()->type_checks() ? 1 : 0); compiler->AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id(), token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); if (compiler->is_optimizing()) { __ Drop1(); } @@ -280,7 +280,7 @@ EMIT_NATIVE_CODE(PolymorphicInstanceCall, __ StaticCall(instance_call()->ArgumentCount(), argdesc_kidx); compiler->AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id(), instance_call()->token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); __ PopLocal(locs()->out(0).reg()); } @@ -301,7 +301,7 @@ EMIT_NATIVE_CODE(CheckStackOverflow, } compiler->AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id(), token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kNoResult); } @@ -431,7 +431,7 @@ EMIT_NATIVE_CODE(InitStaticField, } else { __ InitStaticTOS(); } - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kNoResult); } @@ -449,7 +449,7 @@ EMIT_NATIVE_CODE(ClosureCall, const intptr_t argdesc_kidx = compiler->assembler()->AddConstant(arguments_descriptor); __ StaticCall(argument_count, argdesc_kidx); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); if (compiler->is_optimizing()) { __ PopLocal(locs()->out(0).reg()); } @@ -699,11 +699,11 @@ EMIT_NATIVE_CODE(CreateArray, __ Push(type_arguments); __ Push(length); __ CreateArrayTOS(); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); __ PopLocal(out); } else { __ CreateArrayTOS(); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); } } @@ -917,7 +917,7 @@ EMIT_NATIVE_CODE(StringInterpolate, __ PushConstant(CallFunction()); const intptr_t argdesc_kidx = __ AddConstant(arguments_descriptor); __ StaticCall(kArgumentCount, argdesc_kidx); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kHasResult); if (compiler->is_optimizing()) { __ PopLocal(locs()->out(0).reg()); } @@ -1176,7 +1176,7 @@ EMIT_NATIVE_CODE(Throw, 0, Location::NoLocation(), LocationSummary::kCall) { __ Throw(0); compiler->AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id(), token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kNoResult); __ Trap(); } @@ -1186,7 +1186,7 @@ EMIT_NATIVE_CODE(ReThrow, 0, Location::NoLocation(), LocationSummary::kCall) { __ Throw(1); compiler->AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id(), token_pos()); - compiler->RecordAfterCall(this); + compiler->RecordAfterCall(this, FlowGraphCompiler::kNoResult); __ Trap(); } diff --git a/runtime/vm/simulator_dbc.cc b/runtime/vm/simulator_dbc.cc index dea082ab612..b114dc6c83c 100644 --- a/runtime/vm/simulator_dbc.cc +++ b/runtime/vm/simulator_dbc.cc @@ -3554,19 +3554,13 @@ RawObject* Simulator::Call(const Code& code, { BYTECODE(Deopt, A_D); + + // Note: frame translation will take care of preserving result at the + // top of the stack. See CompilerDeoptInfo::CreateDeoptInfo. const bool is_lazy = rD == 0; - // Preserve result of the previous call. - // TODO(vegorov) we could have actually included result into the - // deoptimization environment because it is passed through the stack. - // If we do then we could remove special result handling from this code. - RawObject* result = SP[0]; - - // When not preserving the result, we still need to preserve SP[0] as it - // contains some temporary expression. - if (!is_lazy) { - SP++; - } + // Make sure we preserve SP[0] when entering synthetic frame below. + SP++; // Leaf runtime function DeoptimizeCopyFrame expects a Dart frame. // The code in this frame may not cause GC. @@ -3582,9 +3576,6 @@ RawObject* Simulator::Call(const Code& code, // We are now inside a valid frame. { - if (is_lazy) { - *++SP = result; // Preserve result (call below can cause GC). - } *++SP = 0; // Space for the result: number of materialization args. Exit(thread, FP, SP + 1, /*pc=*/0); NativeArguments native_args(thread, 0, SP, SP); @@ -3592,10 +3583,6 @@ RawObject* Simulator::Call(const Code& code, } const intptr_t materialization_arg_count = Smi::Value(RAW_CAST(Smi, *SP--)) / kWordSize; - if (is_lazy) { - // Reload the result. It might have been relocated by GC. - result = *SP--; - } // Restore caller PC. pc = SavedCallerPC(FP); @@ -3604,21 +3591,13 @@ RawObject* Simulator::Call(const Code& code, // Check if it is a fake PC marking the entry frame. ASSERT((reinterpret_cast(pc) & 2) == 0); - // Restore SP, FP and PP. Push result and dispatch. - // Note: unlike in a normal return sequence we don't need to drop - // arguments - those are not part of the innermost deoptimization - // environment they were dropped by FlowGraphCompiler::RecordAfterCall. - - // If the result is not preserved, the unoptimized frame ends at the - // next slot. - SP = FrameArguments(FP, materialization_arg_count); + // Restore SP, FP and PP. + // Unoptimized frame SP is one below FrameArguments(...) because + // FrameArguments(...) returns a pointer to the first argument. + SP = FrameArguments(FP, materialization_arg_count) - 1; FP = SavedCallerFP(FP); pp = SimulatorHelpers::FrameCode(FP)->ptr()->object_pool_->ptr(); - if (is_lazy) { - SP[0] = result; // Put the result on the stack. - } else { - SP--; // No result to push. - } + DISPATCH(); } diff --git a/tests/language/vm/lazy_deopt_vm_test.dart b/tests/language/vm/lazy_deopt_vm_test.dart new file mode 100644 index 00000000000..3f4b3923459 --- /dev/null +++ b/tests/language/vm/lazy_deopt_vm_test.dart @@ -0,0 +1,39 @@ +// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +// Test deoptimization on an optimistically hoisted smi check. +// VMOptions=--deoptimize_every=10 --optimization-counter-threshold=10 --no-background-compilation --enable-inlining-annotations + +// Test that lazy deoptimization on stack checks does not damage unoptimized +// frame. + +import 'package:expect/expect.dart'; + + +foo() { + var a = 0; + var b = 1; + var c = 2; + var d = 3; + var e = 4; + for (var i = 0; i < 10; i++) { + a++; + b++; + c++; + d++; + e++; + } + Expect.equals(10, a); + Expect.equals(11, b); + Expect.equals(12, c); + Expect.equals(13, d); + Expect.equals(14, e); +} + +main() { + for (var i = 0; i < 10; ++i) { + foo(); + } +} + +