From 0a4047a69b0f2b4b16d0a79f4c4db2aec506c18d Mon Sep 17 00:00:00 2001
From: jarin <jarin@chromium.org>
Date: Tue, 17 Feb 2015 07:24:28 -0800
Subject: [PATCH] During arguments materialization, do not store materialized
 objects without lazy deopt.

BUG=
R=mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/919173003

Cr-Commit-Position: refs/heads/master@{#26695}
---
 src/deoptimizer.cc                                 | 18 +++++++++++-------
 src/deoptimizer.h                                  |  1 +
 .../regress/regress-arg-materialize-store.js       | 22 ++++++++++++++++++++++
 3 files changed, 34 insertions(+), 7 deletions(-)
 create mode 100644 test/mjsunit/regress/regress-arg-materialize-store.js

diff --git a/src/deoptimizer.cc b/src/deoptimizer.cc
index 7c79c89..df8e5cf 100644
--- a/src/deoptimizer.cc
+++ b/src/deoptimizer.cc
@@ -3190,7 +3190,10 @@ SlotRef SlotRefValueBuilder::ComputeSlotForNextArgument(
 SlotRefValueBuilder::SlotRefValueBuilder(JavaScriptFrame* frame,
                                          int inlined_jsframe_index,
                                          int formal_parameter_count)
-    : current_slot_(0), args_length_(-1), first_slot_index_(-1) {
+    : current_slot_(0),
+      args_length_(-1),
+      first_slot_index_(-1),
+      should_deoptimize_(false) {
   DisallowHeapAllocation no_gc;
 
   int deopt_index = Safepoint::kNoDeoptimizationIndex;
@@ -3208,7 +3211,6 @@ SlotRefValueBuilder::SlotRefValueBuilder(JavaScriptFrame* frame,
   CHECK_GT(jsframe_count, inlined_jsframe_index);
   int jsframes_to_skip = inlined_jsframe_index;
   int number_of_slots = -1;  // Number of slots inside our frame (yet unknown)
-  bool should_deopt = false;
   while (number_of_slots != 0) {
     opcode = static_cast<Translation::Opcode>(it.Next());
     bool processed = false;
@@ -3265,7 +3267,7 @@ SlotRefValueBuilder::SlotRefValueBuilder(JavaScriptFrame* frame,
         number_of_slots += slot.GetChildrenCount();
         if (slot.Representation() == SlotRef::DEFERRED_OBJECT ||
             slot.Representation() == SlotRef::DUPLICATE_OBJECT) {
-          should_deopt = true;
+          should_deoptimize_ = true;
         }
       }
 
@@ -3276,7 +3278,7 @@ SlotRefValueBuilder::SlotRefValueBuilder(JavaScriptFrame* frame,
       it.Skip(Translation::NumberOfOperandsFor(opcode));
     }
   }
-  if (should_deopt) {
+  if (should_deoptimize_) {
     List<JSFunction*> functions(2);
     frame->GetFunctions(&functions);
     Deoptimizer::DeoptimizeFunction(functions[0]);
@@ -3492,9 +3494,11 @@ void SlotRefValueBuilder::Finish(Isolate* isolate) {
   // We should have processed all the slots
   CHECK_EQ(slot_refs_.length(), current_slot_);
 
-  if (materialized_objects_.length() > prev_materialized_count_) {
-    // We have materialized some new objects, so we have to store them
-    // to prevent duplicate materialization
+  if (should_deoptimize_ &&
+      materialized_objects_.length() > prev_materialized_count_) {
+    // We have materialized some new objects and they might be accessible
+    // from the arguments object, so we have to store them
+    // to prevent duplicate materialization.
     Handle<FixedArray> array = isolate->factory()->NewFixedArray(
         materialized_objects_.length());
     for (int i = 0; i < materialized_objects_.length(); i++) {
diff --git a/src/deoptimizer.h b/src/deoptimizer.h
index 91b6fca..471a05d 100644
--- a/src/deoptimizer.h
+++ b/src/deoptimizer.h
@@ -939,6 +939,7 @@ class SlotRefValueBuilder BASE_EMBEDDED {
   int current_slot_;
   int args_length_;
   int first_slot_index_;
+  bool should_deoptimize_;
 
   static SlotRef ComputeSlotForNextArgument(
       Translation::Opcode opcode,
diff --git a/test/mjsunit/regress/regress-arg-materialize-store.js b/test/mjsunit/regress/regress-arg-materialize-store.js
new file mode 100644
index 0000000..2a30dc8
--- /dev/null
+++ b/test/mjsunit/regress/regress-arg-materialize-store.js
@@ -0,0 +1,22 @@
+// Copyright 2015 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
+
+function f() {
+  return f.arguments;
+}
+
+function g(deopt) {
+  var o = { x : 2 };
+  f();
+  o.x = 1;
+  deopt + 0;
+  return o.x;
+}
+
+g(0);
+g(0);
+%OptimizeFunctionOnNextCall(g);
+assertEquals(1, g({}));
-- 
2.7.4