Revision: 9083
Author: [email protected]
Date: Wed Aug 31 06:26:08 2011
Log: Introduce local function declarations in Crankshaft and fix issue
1647.
We have to emit code for declarations later into the body block
(and not into the start block) so that the environment contains
the correct values.
In order to capture the environment effect of the declarations
that generate code (function declarations) I inserted a separate
AST id and a HSimulate after the declarations are visited.
Also fixes handling deopt in named function expressions:
BUG=v8:1647
TEST=test/mjsunit/regress/regress-fundecl.js,
test/mjsunit/regress/regress-1647.js
Review URL: http://codereview.chromium.org/7776009
http://code.google.com/p/v8/source/detail?r=9083
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-1647.js
/branches/bleeding_edge/test/mjsunit/regress/regress-fundecl.js
Modified:
/branches/bleeding_edge/src/arm/full-codegen-arm.cc
/branches/bleeding_edge/src/ast.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/mips/full-codegen-mips.cc
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1647.js Wed Aug 31
06:26:08 2011
@@ -0,0 +1,43 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+// Test for correct deoptimization in named function expressions.
+
+var t = { foo: function() {} };
+
+var f = (function bar() {
+ t.foo();
+ assertEquals("function", typeof bar);
+});
+
+for (var i = 0; i < 10; i++) f();
+%OptimizeFunctionOnNextCall(f);
+t.number = 2;
+f();
+
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-fundecl.js Wed Aug
31 06:26:08 2011
@@ -0,0 +1,44 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+// Test hoisting of function declarations in the optimizing
+// compiler in case of deoptimization.
+
+function h(a, b) {
+ var r = a + b;
+ function X() { return 42; }
+ return r + X();
+}
+
+for (var i = 0; i < 5; i++) h(1,2);
+
+%OptimizeFunctionOnNextCall(h);
+
+assertEquals(45, h(1,2));
+assertEquals("foo742", h("foo", 7));
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Tue Aug 30 04:23:57
2011
+++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Wed Aug 31 06:26:08
2011
@@ -258,6 +258,7 @@
scope()->VisitIllegalRedeclaration(this);
} else {
+ PrepareForBailoutForId(AstNode::kFunctionEntryId, NO_REGISTERS);
{ Comment cmnt(masm_, "[ Declarations");
// For named function expressions, declare the function name as a
// constant.
@@ -268,7 +269,7 @@
}
{ Comment cmnt(masm_, "[ Stack check");
- PrepareForBailoutForId(AstNode::kFunctionEntryId, NO_REGISTERS);
+ PrepareForBailoutForId(AstNode::kDeclarationsId, NO_REGISTERS);
Label ok;
__ LoadRoot(ip, Heap::kStackLimitRootIndex);
__ cmp(sp, Operand(ip));
=======================================
--- /branches/bleeding_edge/src/ast.h Mon Aug 29 00:07:39 2011
+++ /branches/bleeding_edge/src/ast.h Wed Aug 31 06:26:08 2011
@@ -134,6 +134,10 @@
static const int kNoNumber = -1;
static const int kFunctionEntryId = 2; // Using 0 could disguise errors.
+ // This AST id identifies the point after the declarations have been
+ // visited. We need it to capture the environment effects of declarations
+ // that emit code (function declarations).
+ static const int kDeclarationsId = 3;
// Override ZoneObject's new to count allocated AST nodes.
void* operator new(size_t size, Zone* zone) {
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Tue Aug 30 04:23:57 2011
+++ /branches/bleeding_edge/src/hydrogen.cc Wed Aug 31 06:26:08 2011
@@ -2273,10 +2273,6 @@
return NULL;
}
SetupScope(scope);
- VisitDeclarations(scope->declarations());
- HValue* context = environment()->LookupContext();
- AddInstruction(
- new(zone()) HStackCheck(context, HStackCheck::kFunctionEntry));
// Add an edge to the body entry. This is warty: the graph's start
// environment will be used by the Lithium translation as the initial
@@ -2298,6 +2294,23 @@
current_block()->Goto(body_entry);
body_entry->SetJoinId(AstNode::kFunctionEntryId);
set_current_block(body_entry);
+
+ // Handle implicit declaration of the function name in named function
+ // expressions before other declarations.
+ if (scope->is_function_scope() && scope->function() != NULL) {
+ if (!scope->function()->IsStackAllocated()) {
+ Bailout("unsupported declaration");
+ return NULL;
+ }
+ environment()->Bind(scope->function(), graph()->GetConstantHole());
+ }
+ VisitDeclarations(scope->declarations());
+ AddSimulate(AstNode::kDeclarationsId);
+
+ HValue* context = environment()->LookupContext();
+ AddInstruction(
+ new(zone()) HStackCheck(context, HStackCheck::kFunctionEntry));
+
VisitStatements(info()->function()->body());
if (HasStackOverflow()) return NULL;
@@ -5810,7 +5823,6 @@
// We support only declarations that do not require code generation.
Variable* var = decl->proxy()->var();
if (!var->IsStackAllocated() ||
- decl->fun() != NULL ||
decl->mode() == Variable::LET) {
return Bailout("unsupported declaration");
}
@@ -5818,6 +5830,10 @@
if (decl->mode() == Variable::CONST) {
ASSERT(var->IsStackAllocated());
environment()->Bind(var, graph()->GetConstantHole());
+ } else if (decl->fun() != NULL) {
+ VisitForValue(decl->fun());
+ HValue* function = Pop();
+ environment()->Bind(var, function);
}
}
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Tue Aug 30
04:23:57 2011
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Wed Aug 31
06:26:08 2011
@@ -255,6 +255,7 @@
scope()->VisitIllegalRedeclaration(this);
} else {
+ PrepareForBailoutForId(AstNode::kFunctionEntryId, NO_REGISTERS);
{ Comment cmnt(masm_, "[ Declarations");
// For named function expressions, declare the function name as a
// constant.
@@ -265,7 +266,7 @@
}
{ Comment cmnt(masm_, "[ Stack check");
- PrepareForBailoutForId(AstNode::kFunctionEntryId, NO_REGISTERS);
+ PrepareForBailoutForId(AstNode::kDeclarationsId, NO_REGISTERS);
Label ok;
ExternalReference stack_limit =
ExternalReference::address_of_stack_limit(isolate());
=======================================
--- /branches/bleeding_edge/src/mips/full-codegen-mips.cc Wed Aug 31
03:01:43 2011
+++ /branches/bleeding_edge/src/mips/full-codegen-mips.cc Wed Aug 31
06:26:08 2011
@@ -266,6 +266,7 @@
scope()->VisitIllegalRedeclaration(this);
} else {
+ PrepareForBailoutForId(AstNode::kFunctionEntryId, NO_REGISTERS);
{ Comment cmnt(masm_, "[ Declarations");
// For named function expressions, declare the function name as a
// constant.
@@ -276,7 +277,7 @@
}
{ Comment cmnt(masm_, "[ Stack check");
- PrepareForBailoutForId(AstNode::kFunctionEntryId, NO_REGISTERS);
+ PrepareForBailoutForId(AstNode::kDeclarationsId, NO_REGISTERS);
Label ok;
__ LoadRoot(t0, Heap::kStackLimitRootIndex);
__ Branch(&ok, hs, sp, Operand(t0));
=======================================
--- /branches/bleeding_edge/src/parser.cc Tue Aug 30 04:23:57 2011
+++ /branches/bleeding_edge/src/parser.cc Wed Aug 31 06:26:08 2011
@@ -532,7 +532,7 @@
parser->top_scope_ = scope;
parser->lexical_scope_ = this;
parser->with_nesting_level_ = 0;
- isolate->set_ast_node_id(AstNode::kFunctionEntryId + 1);
+ isolate->set_ast_node_id(AstNode::kDeclarationsId + 1);
}
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Tue Aug 30 04:23:57
2011
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Wed Aug 31 06:26:08
2011
@@ -245,6 +245,7 @@
Comment cmnt(masm_, "[ Declarations");
scope()->VisitIllegalRedeclaration(this);
} else {
+ PrepareForBailoutForId(AstNode::kFunctionEntryId, NO_REGISTERS);
{ Comment cmnt(masm_, "[ Declarations");
// For named function expressions, declare the function name as a
// constant.
@@ -255,7 +256,7 @@
}
{ Comment cmnt(masm_, "[ Stack check");
- PrepareForBailoutForId(AstNode::kFunctionEntryId, NO_REGISTERS);
+ PrepareForBailoutForId(AstNode::kDeclarationsId, NO_REGISTERS);
Label ok;
__ CompareRoot(rsp, Heap::kStackLimitRootIndex);
__ j(above_equal, &ok, Label::kNear);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
