Reviewers: Jakob,
Message:
PTAL
Description:
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
Please review this at https://codereview.chromium.org/736043002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+85, -14 lines):
M src/hydrogen.h
M src/hydrogen.cc
A test/mjsunit/regress/regress-3709.js
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
003f4aefeee384ae8847cb3ece5576d56adce4ec..942b3a80859de92e8db71c7e8c535951df2e1993
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -4074,8 +4074,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);
}
@@ -4301,6 +4305,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);
@@ -8885,13 +8897,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, true)) 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;
}
@@ -9178,6 +9185,20 @@ bool
HOptimizedGraphBuilder::TryHandleArrayCallNew(CallNew* expr,
}
+bool HOptimizedGraphBuilder::CanBeFunctionApplyArguments(
+ Call* expr, bool mark_arguments_as_live) {
+ 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 = mark_arguments_as_live
+ ? LookupAndMakeLive(arg_two->var())
+ : Lookup(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);
@@ -9214,13 +9235,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);
@@ -9256,7 +9278,18 @@ void HOptimizedGraphBuilder::VisitCall(Call* expr) {
}
} else {
- CHECK_ALIVE(VisitExpressions(expr->arguments()));
+ ArgumentsAllowedFlag arguments_flag = ARGUMENTS_NOT_ALLOWED;
+ if (CanBeFunctionApplyArguments(expr, false)) {
+ 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);
Index: src/hydrogen.h
diff --git a/src/hydrogen.h b/src/hydrogen.h
index
43388464cb0d9d03b588730213b99332ca4b1acc..164a06a5f3ee15114aee746fae3bc70a64a67ff5
100644
--- 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
};
@@ -2261,7 +2262,11 @@ class HOptimizedGraphBuilder : public HGraphBuilder,
public AstVisitor {
#endif
}
}
-
+ HValue* Lookup(Variable* var) {
+ HEnvironment* env = environment();
+ int index = env->IndexFor(var);
+ return env->Lookup(index);
+ }
HValue* LookupAndMakeLive(Variable* var) {
HEnvironment* env = environment();
int index = env->IndexFor(var);
@@ -2290,6 +2295,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.
@@ -2723,6 +2730,8 @@ class HOptimizedGraphBuilder : public HGraphBuilder,
public AstVisitor {
HInstruction* BuildCallConstantFunction(Handle<JSFunction> target,
int argument_count);
+ bool CanBeFunctionApplyArguments(Call* expr, bool
mark_arguments_as_live);
+
// The translation state of the currently-being-translated function.
FunctionState* function_state_;
Index: test/mjsunit/regress/regress-3709.js
diff --git a/test/mjsunit/regress/regress-3709.js
b/test/mjsunit/regress/regress-3709.js
new file mode 100644
index
0000000000000000000000000000000000000000..f4e820e528807a098c4f6863c8baca4190710b47
--- /dev/null
+++ b/test/mjsunit/regress/regress-3709.js
@@ -0,0 +1,29 @@
+// 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();
+if (%GetOptimizationStatus(foo) != 4) {
+ assertEquals(1, %GetOptimizationStatus(foo));
+ foo(10);
+ assertEquals(2, %GetOptimizationStatus(foo));
+}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
