diff --git a/runtime/tests/vm/dart/deopt/restart_call_on_deopt_regress_46070_test.dart b/runtime/tests/vm/dart/deopt/restart_call_on_deopt_regress_46070_test.dart new file mode 100644 index 00000000000..959c792264d --- /dev/null +++ b/runtime/tests/vm/dart/deopt/restart_call_on_deopt_regress_46070_test.dart @@ -0,0 +1,22 @@ +// Copyright (c) 2021, 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. + +// VMOptions=--deterministic --deoptimize-on-runtime-call-every=3 --optimization-counter-threshold=10 + +main() { + final l = [1, 2, 3, 4, 5]; + for (int i = 0; i < 1000; ++i) { + if (sumIt(l) != 15) throw 'failed'; + } +} + +@pragma('vm:never-inline') +int sumIt(dynamic arg) { + int sum = 0; + for (int i = 0; i < 5; ++i) { + final l = arg as List; + sum += l[i]; + } + return sum; +} diff --git a/runtime/tests/vm/dart_2/deopt/restart_call_on_deopt_regress_46070_test.dart b/runtime/tests/vm/dart_2/deopt/restart_call_on_deopt_regress_46070_test.dart new file mode 100644 index 00000000000..959c792264d --- /dev/null +++ b/runtime/tests/vm/dart_2/deopt/restart_call_on_deopt_regress_46070_test.dart @@ -0,0 +1,22 @@ +// Copyright (c) 2021, 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. + +// VMOptions=--deterministic --deoptimize-on-runtime-call-every=3 --optimization-counter-threshold=10 + +main() { + final l = [1, 2, 3, 4, 5]; + for (int i = 0; i < 1000; ++i) { + if (sumIt(l) != 15) throw 'failed'; + } +} + +@pragma('vm:never-inline') +int sumIt(dynamic arg) { + int sum = 0; + for (int i = 0; i < 5; ++i) { + final l = arg as List; + sum += l[i]; + } + return sum; +} diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.cc b/runtime/vm/compiler/backend/flow_graph_compiler.cc index bc3bf363670..0094d89d08a 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler.cc @@ -501,10 +501,20 @@ void FlowGraphCompiler::EmitCallsiteMetadata(const InstructionSource& source, if ((deopt_id != DeoptId::kNone) && !FLAG_precompiled_mode) { // Marks either the continuation point in unoptimized code or the // deoptimization point in optimized code, after call. - const intptr_t deopt_id_after = DeoptId::ToDeoptAfter(deopt_id); if (is_optimizing()) { - AddDeoptIndexAtCall(deopt_id_after, env); + ASSERT(env != nullptr); + // Note that we may lazy-deopt to the same IR instruction in unoptimized + // code or to another IR instruction (e.g. if LICM hoisted an instruction + // it will lazy-deopt to a Goto). + // If we happen to deopt to the beginning of an instruction in unoptimized + // code, we'll use the before deopt-id, otherwise the after deopt-id. + const intptr_t dest_deopt_id = env->LazyDeoptToBeforeDeoptId() + ? deopt_id + : DeoptId::ToDeoptAfter(deopt_id); + AddDeoptIndexAtCall(dest_deopt_id, env); } else { + ASSERT(env == nullptr); + const intptr_t deopt_id_after = DeoptId::ToDeoptAfter(deopt_id); // Add deoptimization continuation point after the call and before the // arguments are removed. AddCurrentDescriptor(UntaggedPcDescriptors::kDeopt, deopt_id_after, diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc index 55da50a5ca7..0986d22b22f 100644 --- a/runtime/vm/compiler/backend/il.cc +++ b/runtime/vm/compiler/backend/il.cc @@ -5742,6 +5742,7 @@ Environment* Environment::DeepCopy(Zone* zone, intptr_t length) const { length, fixed_parameter_count_, LazyDeoptPruneCount(), parsed_function_, (outer_ == NULL) ? NULL : outer_->DeepCopy(zone)); copy->SetDeoptId(DeoptIdBits::decode(bitfield_)); + copy->SetLazyDeoptToBeforeDeoptId(LazyDeoptToBeforeDeoptId()); if (locations_ != NULL) { Location* new_locations = zone->Alloc(length); copy->set_locations(new_locations); diff --git a/runtime/vm/compiler/backend/il.h b/runtime/vm/compiler/backend/il.h index a0b3cd1b6f6..b41c986f88f 100644 --- a/runtime/vm/compiler/backend/il.h +++ b/runtime/vm/compiler/backend/il.h @@ -9423,6 +9423,14 @@ class Environment : public ZoneAllocated { return LazyDeoptPruningBits::decode(bitfield_); } + bool LazyDeoptToBeforeDeoptId() const { + return LazyDeoptToBeforeDeoptId::decode(bitfield_); + } + + void MarkAsLazyDeoptToBeforeDeoptId() { + bitfield_ = LazyDeoptToBeforeDeoptId::update(true, bitfield_); + } + Environment* GetLazyDeoptEnv(Zone* zone) { const intptr_t num_args_to_prune = LazyDeoptPruneCount(); if (num_args_to_prune == 0) return this; @@ -9501,11 +9509,13 @@ class Environment : public ZoneAllocated { friend class FlowGraphDeserializer; // For constructor and deopt_id_. class LazyDeoptPruningBits : public BitField {}; + class LazyDeoptToBeforeDeoptId + : public BitField {}; class DeoptIdBits : public BitField {}; Environment(intptr_t length, @@ -9516,6 +9526,7 @@ class Environment : public ZoneAllocated { : values_(length), fixed_parameter_count_(fixed_parameter_count), bitfield_(DeoptIdBits::encode(DeoptId::kNone) | + LazyDeoptToBeforeDeoptId::encode(false) | LazyDeoptPruningBits::encode(lazy_deopt_pruning_count)), parsed_function_(parsed_function), outer_(outer) {} @@ -9526,6 +9537,9 @@ class Environment : public ZoneAllocated { void SetLazyDeoptPruneCount(intptr_t value) { bitfield_ = LazyDeoptPruningBits::update(value, bitfield_); } + void SetLazyDeoptToBeforeDeoptId(bool value) { + bitfield_ = LazyDeoptToBeforeDeoptId::update(value, bitfield_); + } GrowableArray values_; Location* locations_ = nullptr; diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc index 5c8dcc807cd..d576993651d 100644 --- a/runtime/vm/compiler/backend/inliner.cc +++ b/runtime/vm/compiler/backend/inliner.cc @@ -2657,6 +2657,11 @@ static bool InlineSetIndexed(FlowGraph* flow_graph, Symbols::Value(), call->deopt_id()); cursor = flow_graph->AppendTo(cursor, assert_value, call->env(), FlowGraph::kValue); + // The environment is that of the InstanceCall([]=, ..., ). + // A lazy-deopt of the inserted AssertAssignable must continue in + // unoptimzed code. + // => We will re-try this []= call in unoptimized code. + assert_value->env()->MarkAsLazyDeoptToBeforeDeoptId(); } } diff --git a/runtime/vm/compiler/backend/redundancy_elimination.cc b/runtime/vm/compiler/backend/redundancy_elimination.cc index 5cbc45ee28b..bda4322bf69 100644 --- a/runtime/vm/compiler/backend/redundancy_elimination.cc +++ b/runtime/vm/compiler/backend/redundancy_elimination.cc @@ -1368,6 +1368,9 @@ void LICM::Hoist(ForwardInstructionIterator* it, GotoInstr* last = pre_header->last_instruction()->AsGoto(); // Using kind kEffect will not assign a fresh ssa temporary index. flow_graph()->InsertBefore(last, current, last->env(), FlowGraph::kEffect); + // If the hoisted instruction lazy-deopts, it should continue at the start of + // the Goto (of which we copy the deopt-id from). + current->env()->MarkAsLazyDeoptToBeforeDeoptId(); current->CopyDeoptIdFrom(*last); }