Do not bailout from optimizing functions that use f(x, arguments)

if there is not enough type-feedback to detect that f is Function.prototype.apply.

BUG=v8:3709
LOG=N
TEST=mjsunit/regress/regress-3709

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

Cr-Commit-Position: refs/heads/master@{#25447}

diff --git a/src/ast.h b/src/ast.h
index 3e12d54ff81a4dadf7faffb4833a9129ad485eb0..f4742f155b418cc8dee66b73753cf7f9706ae12c 100644 (file)
--- a/src/ast.h
+++ b/src/ast.h
@@ -1879,6 +1879,13 @@ class Call FINAL : public Expression {
   BailoutId ReturnId() const { return BailoutId(local_id(0)); }
   BailoutId EvalOrLookupId() const { return BailoutId(local_id(1)); }
 
+  bool is_uninitialized() const {
+    return IsUninitializedField::decode(bit_field_);
+  }
+  void set_is_uninitialized(bool b) {
+    bit_field_ = IsUninitializedField::update(bit_field_, b);
+  }
+
   enum CallType {
     POSSIBLY_EVAL_CALL,
     GLOBAL_CALL,
@@ -1903,7 +1910,8 @@ class Call FINAL : public Expression {
       : Expression(zone, pos),
         call_feedback_slot_(FeedbackVectorICSlot::Invalid()),
         expression_(expression),
-        arguments_(arguments) {
+        arguments_(arguments),
+        bit_field_(IsUninitializedField::encode(false)) {
     if (expression->IsProperty()) {
       expression->AsProperty()->mark_for_call();
     }
@@ -1919,6 +1927,8 @@ class Call FINAL : public Expression {
   Handle<JSFunction> target_;
   Handle<Cell> cell_;
   Handle<AllocationSite> allocation_site_;
+  class IsUninitializedField : public BitField8<bool, 0, 1> {};
+  uint8_t bit_field_;
 };
 
 

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 21ef8c019fb410b26214f1751c53e27e3a8579fb..46a3810c75e840f16b9039d86ab9219962e761df 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -4050,8 +4050,12 @@ void EffectContext::ReturnValue(HValue* value) {
 void ValueContext::ReturnValue(HValue* value) {
   // The value is tracked in the bailout environment, and communicated
   // through the environment as the result of the expression.
-  if (!arguments_allowed() && value->CheckFlag(HValue::kIsArguments)) {
-    owner()->Bailout(kBadValueContextForArgumentsValue);
+  if (value->CheckFlag(HValue::kIsArguments)) {
+    if (flag_ == ARGUMENTS_FAKED) {
+      value = owner()->graph()->GetConstantUndefined();
+    } else if (!arguments_allowed()) {
+      owner()->Bailout(kBadValueContextForArgumentsValue);
+    }
   }
   owner()->Push(value);
 }
@@ -4277,6 +4281,14 @@ void HOptimizedGraphBuilder::VisitExpressions(
 }
 
 
+void HOptimizedGraphBuilder::VisitExpressions(ZoneList<Expression*>* exprs,
+                                              ArgumentsAllowedFlag flag) {
+  for (int i = 0; i < exprs->length(); ++i) {
+    CHECK_ALIVE(VisitForValue(exprs->at(i), flag));
+  }
+}
+
+
 bool HOptimizedGraphBuilder::BuildGraph() {
   if (current_info()->function()->is_generator()) {
     Bailout(kFunctionIsAGenerator);
@@ -8861,13 +8873,8 @@ bool HOptimizedGraphBuilder::TryIndirectCall(Call* expr) {
       // is supported.
       if (current_info()->scope()->arguments() == NULL) return false;
 
-      ZoneList<Expression*>* args = expr->arguments();
-      if (args->length() != 2) return false;
+      if (!CanBeFunctionApplyArguments(expr)) return false;
 
-      VariableProxy* arg_two = args->at(1)->AsVariableProxy();
-      if (arg_two == NULL || !arg_two->var()->IsStackAllocated()) return false;
-      HValue* arg_two_value = LookupAndMakeLive(arg_two->var());
-      if (!arg_two_value->CheckFlag(HValue::kIsArguments)) return false;
       BuildFunctionApply(expr);
       return true;
     }
@@ -9154,6 +9161,17 @@ bool HOptimizedGraphBuilder::TryHandleArrayCallNew(CallNew* expr,
 }
 
 
+bool HOptimizedGraphBuilder::CanBeFunctionApplyArguments(Call* expr) {
+  ZoneList<Expression*>* args = expr->arguments();
+  if (args->length() != 2) return false;
+  VariableProxy* arg_two = args->at(1)->AsVariableProxy();
+  if (arg_two == NULL || !arg_two->var()->IsStackAllocated()) return false;
+  HValue* arg_two_value = LookupAndMakeLive(arg_two->var());
+  if (!arg_two_value->CheckFlag(HValue::kIsArguments)) return false;
+  return true;
+}
+
+
 void HOptimizedGraphBuilder::VisitCall(Call* expr) {
   DCHECK(!HasStackOverflow());
   DCHECK(current_block() != NULL);
@@ -9190,13 +9208,14 @@ void HOptimizedGraphBuilder::VisitCall(Call* expr) {
 
     if (FLAG_hydrogen_track_positions) SetSourcePosition(expr->position());
 
-    // Push the function under the receiver.
-    environment()->SetExpressionStackAt(0, function);
 
-    Push(receiver);
 
     if (function->IsConstant() &&
         HConstant::cast(function)->handle(isolate())->IsJSFunction()) {
+      // Push the function under the receiver.
+      environment()->SetExpressionStackAt(0, function);
+      Push(receiver);
+
       Handle<JSFunction> known_function = Handle<JSFunction>::cast(
           HConstant::cast(function)->handle(isolate()));
       expr->set_target(known_function);
@@ -9232,7 +9251,20 @@ void HOptimizedGraphBuilder::VisitCall(Call* expr) {
       }
 
     } else {
-      CHECK_ALIVE(VisitExpressions(expr->arguments()));
+      ArgumentsAllowedFlag arguments_flag = ARGUMENTS_NOT_ALLOWED;
+      if (CanBeFunctionApplyArguments(expr) && expr->is_uninitialized()) {
+        // We have to use EAGER deoptimization here because Deoptimizer::SOFT
+        // gets ignored by the always-opt flag, which leads to incorrect code.
+        Add<HDeoptimize>("Insufficient type feedback for call with arguments",
+                         Deoptimizer::EAGER);
+        arguments_flag = ARGUMENTS_FAKED;
+      }
+
+      // Push the function under the receiver.
+      environment()->SetExpressionStackAt(0, function);
+      Push(receiver);
+
+      CHECK_ALIVE(VisitExpressions(expr->arguments(), arguments_flag));
       CallFunctionFlags flags = receiver->type().IsJSObject()
           ? NO_CALL_FUNCTION_FLAGS : CALL_AS_METHOD;
       call = New<HCallFunction>(function, argument_count, flags);

diff --git a/src/hydrogen.h b/src/hydrogen.h
index 773ea8a3f4ba05522f85180349a4ae2f7b549bf7..acb1ee13dc278844176e20a3ddb478d8c3ae876b 100644 (file)
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -748,7 +748,8 @@ class HOptimizedGraphBuilder;
 
 enum ArgumentsAllowedFlag {
   ARGUMENTS_NOT_ALLOWED,
-  ARGUMENTS_ALLOWED
+  ARGUMENTS_ALLOWED,
+  ARGUMENTS_FAKED
 };
 
 
@@ -2257,7 +2258,6 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
 #endif
     }
   }
-
   HValue* LookupAndMakeLive(Variable* var) {
     HEnvironment* env = environment();
     int index = env->IndexFor(var);
@@ -2286,6 +2286,8 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
 
   // Visit a list of expressions from left to right, each in a value context.
   void VisitExpressions(ZoneList<Expression*>* exprs);
+  void VisitExpressions(ZoneList<Expression*>* exprs,
+                        ArgumentsAllowedFlag flag);
 
   // Remove the arguments from the bailout environment and emit instructions
   // to push them as outgoing parameters.
@@ -2719,6 +2721,8 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   HInstruction* BuildCallConstantFunction(Handle<JSFunction> target,
                                           int argument_count);
 
+  bool CanBeFunctionApplyArguments(Call* expr);
+
   // The translation state of the currently-being-translated function.
   FunctionState* function_state_;
 

diff --git a/src/type-info.cc b/src/type-info.cc
index 2c033b020061bb8479d638d59aea41674075dc9f..9808e9a24ae2b54fecffd489a31f30526b8dd56b 100644 (file)
--- a/src/type-info.cc
+++ b/src/type-info.cc
@@ -89,6 +89,14 @@ bool TypeFeedbackOracle::StoreIsUninitialized(TypeFeedbackId ast_id) {
 }
 
 
+bool TypeFeedbackOracle::CallIsUninitialized(FeedbackVectorICSlot slot) {
+  Handle<Object> value = GetInfo(slot);
+  return value->IsUndefined() ||
+         value.is_identical_to(
+             TypeFeedbackVector::UninitializedSentinel(isolate()));
+}
+
+
 bool TypeFeedbackOracle::CallIsMonomorphic(FeedbackVectorICSlot slot) {
   Handle<Object> value = GetInfo(slot);
   return value->IsAllocationSite() || value->IsJSFunction();

diff --git a/src/type-info.h b/src/type-info.h
index 8fe581813b35cc94e3e3d7dd02d06db4ef4ee4a9..5ec6929a33cc052c63bd9876d1727f4bc41ed4d8 100644 (file)
--- a/src/type-info.h
+++ b/src/type-info.h
@@ -25,6 +25,7 @@ class TypeFeedbackOracle: public ZoneObject {
 
   bool LoadIsUninitialized(TypeFeedbackId id);
   bool StoreIsUninitialized(TypeFeedbackId id);
+  bool CallIsUninitialized(FeedbackVectorICSlot slot);
   bool CallIsMonomorphic(FeedbackVectorICSlot slot);
   bool KeyedArrayCallIsHoley(TypeFeedbackId id);
   bool CallNewIsMonomorphic(FeedbackVectorSlot slot);

diff --git a/src/typing.cc b/src/typing.cc
index 1cfaf64f64a403e4a44b035c889c091625ac0592..7f736a4fc6620419c4dddbf6ca4dc4e32313b9a0 100644 (file)
--- a/src/typing.cc
+++ b/src/typing.cc
@@ -510,12 +510,13 @@ void AstTyper::VisitProperty(Property* expr) {
 void AstTyper::VisitCall(Call* expr) {
   // Collect type feedback.
   RECURSE(Visit(expr->expression()));
+  FeedbackVectorICSlot slot = expr->CallFeedbackSlot();
+  expr->set_is_uninitialized(oracle()->CallIsUninitialized(slot));
   if (!expr->expression()->IsProperty() &&
       expr->IsUsingCallFeedbackSlot(isolate()) &&
-      oracle()->CallIsMonomorphic(expr->CallFeedbackSlot())) {
-    expr->set_target(oracle()->GetCallTarget(expr->CallFeedbackSlot()));
-    Handle<AllocationSite> site =
-        oracle()->GetCallAllocationSite(expr->CallFeedbackSlot());
+      oracle()->CallIsMonomorphic(slot)) {
+    expr->set_target(oracle()->GetCallTarget(slot));
+    Handle<AllocationSite> site = oracle()->GetCallAllocationSite(slot);
     expr->set_allocation_site(site);
   }
 

diff --git a/test/mjsunit/regress/regress-3709.js b/test/mjsunit/regress/regress-3709.js
new file mode 100644 (file)
index 0000000..d2de711
--- /dev/null
+++ b/test/mjsunit/regress/regress-3709.js
@@ -0,0 +1,28 @@
+// Copyright 2014 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 getobj() {
+  return { bar : function() { return 0}};
+}
+
+function foo() {
+  var obj = getobj();
+  var length = arguments.length;
+  if (length == 0) {
+     obj.bar();
+  } else {
+     obj.bar.apply(obj, arguments);
+  }
+}
+
+foo();
+foo();
+%OptimizeFunctionOnNextCall(foo);
+foo();
+assertOptimized(foo);
+foo(10);
+assertUnoptimized(foo);
+%ClearFunctionTypeFeedback(foo);