From 204babf5a0389da311b573cbc254c5b4918ed668 Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Tue, 29 Nov 2016 03:34:07 -0800 Subject: [PATCH] [deoptimizer] Fix deoptimization in {TranslatedState}. This ensures the deoptimization triggered due to materialization of objects by the {TranslatedState} works in conjunction with OSR. The optimized code used for OSR is not installed on the function, hence needs to be specified explicitly when requesting deoptimization for specific stack frames. R=jarin@chromium.org TEST=mjsunit/regress/regress-crbug-668795 BUG=chromium:668795 Review-Url: https://codereview.chromium.org/2534143002 Cr-Commit-Position: refs/heads/master@{#41348} --- src/accessors.cc | 6 +++--- src/deoptimizer.cc | 12 +++++------ src/deoptimizer.h | 7 ++++--- src/runtime/runtime-scopes.cc | 4 +++- test/mjsunit/regress/regress-crbug-668795.js | 21 ++++++++++++++++++++ 5 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-668795.js diff --git a/src/accessors.cc b/src/accessors.cc index aad2b3695e..7c6d344ec0 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -801,8 +801,8 @@ static Handle ArgumentsForInlinedFunction( Handle array = factory->NewFixedArray(argument_count); bool should_deoptimize = false; for (int i = 0; i < argument_count; ++i) { - // If we materialize any object, we should deopt because we might alias - // an object that was eliminated by escape analysis. + // If we materialize any object, we should deoptimize the frame because we + // might alias an object that was eliminated by escape analysis. should_deoptimize = should_deoptimize || iter->IsMaterializedObject(); Handle value = iter->GetValue(); array->set(i, *value); @@ -811,7 +811,7 @@ static Handle ArgumentsForInlinedFunction( arguments->set_elements(*array); if (should_deoptimize) { - translated_values.StoreMaterializedValuesAndDeopt(); + translated_values.StoreMaterializedValuesAndDeopt(frame); } // Return the freshly allocated arguments object. diff --git a/src/deoptimizer.cc b/src/deoptimizer.cc index b7446fd742..94ac61e052 100644 --- a/src/deoptimizer.cc +++ b/src/deoptimizer.cc @@ -391,14 +391,13 @@ void Deoptimizer::MarkAllCodeForContext(Context* context) { } } - -void Deoptimizer::DeoptimizeFunction(JSFunction* function) { +void Deoptimizer::DeoptimizeFunction(JSFunction* function, Code* code) { Isolate* isolate = function->GetIsolate(); RuntimeCallTimerScope runtimeTimer(isolate, &RuntimeCallStats::DeoptimizeCode); TimerEventScope timer(isolate); TRACE_EVENT0("v8", "V8.DeoptimizeCode"); - Code* code = function->code(); + if (code == nullptr) code = function->code(); if (code->kind() == Code::OPTIMIZED_FUNCTION) { // Mark the code for deoptimization and unlink any functions that also // refer to that code. The code cannot be shared across native contexts, @@ -3934,8 +3933,7 @@ TranslatedFrame* TranslatedState::GetArgumentsInfoFromJSFrameIndex( return nullptr; } - -void TranslatedState::StoreMaterializedValuesAndDeopt() { +void TranslatedState::StoreMaterializedValuesAndDeopt(JavaScriptFrame* frame) { MaterializedObjectStore* materialized_store = isolate_->materialized_object_store(); Handle previously_materialized_objects = @@ -3981,8 +3979,8 @@ void TranslatedState::StoreMaterializedValuesAndDeopt() { CHECK(frames_[0].kind() == TranslatedFrame::kFunction || frames_[0].kind() == TranslatedFrame::kInterpretedFunction || frames_[0].kind() == TranslatedFrame::kTailCallerFunction); - Object* const function = frames_[0].front().GetRawValue(); - Deoptimizer::DeoptimizeFunction(JSFunction::cast(function)); + CHECK_EQ(frame->function(), frames_[0].front().GetRawValue()); + Deoptimizer::DeoptimizeFunction(frame->function(), frame->LookupCode()); } } diff --git a/src/deoptimizer.h b/src/deoptimizer.h index 4d84fb76e8..86c177d1af 100644 --- a/src/deoptimizer.h +++ b/src/deoptimizer.h @@ -254,7 +254,7 @@ class TranslatedState { void Prepare(bool has_adapted_arguments, Address stack_frame_pointer); // Store newly materialized values into the isolate. - void StoreMaterializedValuesAndDeopt(); + void StoreMaterializedValuesAndDeopt(JavaScriptFrame* frame); typedef std::vector::iterator iterator; iterator begin() { return frames_.begin(); } @@ -419,8 +419,9 @@ class Deoptimizer : public Malloced { // Deoptimize the function now. Its current optimized code will never be run // again and any activations of the optimized code will get deoptimized when - // execution returns. - static void DeoptimizeFunction(JSFunction* function); + // execution returns. If {code} is specified then the given code is targeted + // instead of the function code (e.g. OSR code not installed on function). + static void DeoptimizeFunction(JSFunction* function, Code* code = nullptr); // Deoptimize all code in the given isolate. static void DeoptimizeAll(Isolate* isolate); diff --git a/src/runtime/runtime-scopes.cc b/src/runtime/runtime-scopes.cc index 603189c4f0..c64aac5bb7 100644 --- a/src/runtime/runtime-scopes.cc +++ b/src/runtime/runtime-scopes.cc @@ -377,6 +377,8 @@ std::unique_ptr[]> GetCallerArguments(Isolate* isolate, NewArray>(*total_argc)); bool should_deoptimize = false; for (int i = 0; i < argument_count; i++) { + // If we materialize any object, we should deoptimize the frame because we + // might alias an object that was eliminated by escape analysis. should_deoptimize = should_deoptimize || iter->IsMaterializedObject(); Handle value = iter->GetValue(); param_data[i] = value; @@ -384,7 +386,7 @@ std::unique_ptr[]> GetCallerArguments(Isolate* isolate, } if (should_deoptimize) { - translated_values.StoreMaterializedValuesAndDeopt(); + translated_values.StoreMaterializedValuesAndDeopt(frame); } return param_data; diff --git a/test/mjsunit/regress/regress-crbug-668795.js b/test/mjsunit/regress/regress-crbug-668795.js new file mode 100644 index 0000000000..a98bad03e5 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-668795.js @@ -0,0 +1,21 @@ +// Copyright 2016 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --ignition + +function g() { + return g.arguments; +} + +function f() { + var result = "R:"; + for (var i = 0; i < 3; ++i) { + if (i == 1) %OptimizeOsr(); + result += g([1])[0]; + result += g([2])[0]; + } + return result; +} + +assertEquals("R:121212", f());