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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9083 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index 408946e449de0e6708ed17523e048538974e102a..1fdc01d7c7e2d598538a7e3f2d23b9cce9d87f4d 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -258,6 +258,7 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     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 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     }
 
     { 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));

diff --git a/src/ast.h b/src/ast.h
index 74182d5dc1cfdd761465ec97d53b5747f3299971..67902cdc32d0f7fa4de0a71c0a69ec878129cd93 100644 (file)
--- a/src/ast.h
+++ b/src/ast.h
@@ -134,6 +134,10 @@ class AstNode: public ZoneObject {
 
   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) {

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 55bf2dc1a5b5407eb89e7e4a3dfc4c24119faea8..facc458553cdd43c11376f7a2b459912c30f8f68 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -2273,10 +2273,6 @@ HGraph* HGraphBuilder::CreateGraph() {
       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 @@ HGraph* HGraphBuilder::CreateGraph() {
     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 @@ void HGraphBuilder::VisitDeclaration(Declaration* decl) {
   // 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 @@ void HGraphBuilder::VisitDeclaration(Declaration* decl) {
   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);
   }
 }
 

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index 52e33d61eb00f801d48414635fe5a6a5f1eb9f9a..253387f42d5346e1a14cf49f040401486af88c39 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -255,6 +255,7 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     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 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     }
 
     { 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());

diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc
index 788fe750ffaf7f4e3fcd0ba9a387a6dcf67a0a8d..92bac9075eb0f520258dda2229cda1cfb4aa7ff7 100644 (file)
--- a/src/mips/full-codegen-mips.cc
+++ b/src/mips/full-codegen-mips.cc
@@ -266,6 +266,7 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     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 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     }
 
     { 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));

diff --git a/src/parser.cc b/src/parser.cc
index 615c6ea029a98f3e22db6737ba197ba82d60b4e9..cf907ce0a5c88d081be37a3b07bafa2e1d1c99ff 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -532,7 +532,7 @@ LexicalScope::LexicalScope(Parser* parser, Scope* scope, Isolate* isolate)
   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);
 }
 
 

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index e77ec649e7351a2eadd86e207921bc4ec0b4a88d..365012cdc57689019c337e35e0b9dee5e04c2085 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -245,6 +245,7 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     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 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     }
 
     { 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);

diff --git a/test/mjsunit/regress/regress-1647.js b/test/mjsunit/regress/regress-1647.js
new file mode 100644 (file)
index 0000000..a6afcc0
--- /dev/null
+++ b/test/mjsunit/regress/regress-1647.js
@@ -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();
+

diff --git a/test/mjsunit/regress/regress-fundecl.js b/test/mjsunit/regress/regress-fundecl.js
new file mode 100644 (file)
index 0000000..fddb589
--- /dev/null
+++ b/test/mjsunit/regress/regress-fundecl.js
@@ -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));