Reviewers: Michael Starzinger,

Description:
Always create HArgumentsObject on function entry.

We do not know if we are going to need it and creating it lazyly might cause us
to insert it at the block that does not dominate all uses.

[email protected]
TEST=mjsunit/compiler/inline-arguments.js


Please review this at https://chromiumcodereview.appspot.com/9692046/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/hydrogen.h
  M src/hydrogen.cc
  M test/mjsunit/compiler/inline-arguments.js


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 6a7b6098cb8537d747f2ce674e41e56fdd0f0c7c..4c4652b452cb1e1ed92c1034d3a881f0b6ca7f9f 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -2613,6 +2613,10 @@ void HGraphBuilder::SetUpScope(Scope* scope) {
   AddInstruction(undefined_constant);
   graph_->set_undefined_constant(undefined_constant);

+  HArgumentsObject* object = new(zone()) HArgumentsObject;
+  AddInstruction(object);
+  graph()->SetArgumentsObject(object);
+
   // Set the initial values of parameters including "this".  "This" has
   // parameter index 0.
   ASSERT_EQ(scope->num_parameters() + 1, environment()->parameter_count());
@@ -2640,11 +2644,6 @@ void HGraphBuilder::SetUpScope(Scope* scope) {
       return Bailout("context-allocated arguments");
     }

-    if (!graph()->HasArgumentsObject()) {
-      HArgumentsObject* object = new(zone()) HArgumentsObject;
-      AddInstruction(object);
-      graph()->SetArgumentsObject(object);
-    }
     environment()->Bind(scope->arguments(),
                         graph()->GetArgumentsObject());
   }
@@ -5325,11 +5324,6 @@ bool HGraphBuilder::TryInline(CallKind call_kind,
   // If the function uses arguments object create and bind one.
   if (function->scope()->arguments() != NULL) {
     ASSERT(function->scope()->arguments()->IsStackAllocated());
-    if (!graph()->HasArgumentsObject()) {
-      HArgumentsObject* object = new(zone()) HArgumentsObject;
-      AddInstruction(object);
-      graph()->SetArgumentsObject(object);
-    }
     environment()->Bind(function->scope()->arguments(),
                         graph()->GetArgumentsObject());
   }
Index: src/hydrogen.h
diff --git a/src/hydrogen.h b/src/hydrogen.h
index b0d67ebb66c61f1aa8b790ddd392bd23edb5430c..919a5e1cbca7d611abf77d67a40579df7c8bf7ab 100644
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -293,7 +293,6 @@ class HGraph: public ZoneObject {
   HArgumentsObject* GetArgumentsObject() const {
     return arguments_object_.get();
   }
-  bool HasArgumentsObject() const { return arguments_object_.is_set(); }

   void SetArgumentsObject(HArgumentsObject* object) {
     arguments_object_.set(object);
Index: test/mjsunit/compiler/inline-arguments.js
diff --git a/test/mjsunit/compiler/inline-arguments.js b/test/mjsunit/compiler/inline-arguments.js index b79f59c4a16e0bc4c28c2ecf8bcff53a1e478d26..9712b2d18352c042247e93abe15645e24030b5c7 100644
--- a/test/mjsunit/compiler/inline-arguments.js
+++ b/test/mjsunit/compiler/inline-arguments.js
@@ -54,3 +54,29 @@ A.prototype.X.apply = function (receiver, args) {
   return Function.prototype.apply.call(this, receiver, args);
 };
 a.Z(4,5,6);
+
+
+// Ensure that HArgumentsObject is inserted in a correct place
+// and dominates all uses.
+function F1() { }
+function F2() { F1.apply(this, arguments); }
+function F3(x, y) {
+  if (x) {
+    F2(y);
+  }
+}
+
+function F31() {
+  return F1.apply(this, arguments);
+}
+
+function F4() {
+  F3(true, false);
+  return F31(1);
+}
+
+F4(1);
+F4(1);
+F4(1);
+%OptimizeFunctionOnNextCall(F4);
+F4(1);


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to