Make closures optimizable by Crankshaft compiler.

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 05a59d26d3dc139ae3ec5aded1c79fb3bda7cfc4..cade33e97f23ce4936e32c507b99d5b5dec00cec 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -2930,6 +2930,22 @@ void HGraphBuilder::VisitVariableProxy(VariableProxy* expr) {
       BAILOUT("unsupported context for arguments object");
     }
     ast_context()->ReturnValue(environment()->Lookup(variable));
+  } else if (variable->AsSlot() != NULL &&
+             variable->AsSlot()->type() == Slot::CONTEXT) {

[Kevin Millikin (Chromium), 2010/12/20 12:05:44]

This predicate (->AsSlot() != NULL && ->AsSlot()->type == Slot::CONTEXT) should
probably be defined on the Variable class as something like "IsInContext()" or
"IsHeapAllocated()".

[antonm, 2010/12/20 20:39:24]

Done.

I named IsContextSlot as IMHO IsHeapAllocated rather leaks implementation detail
and IsInContext looks somewhat vague for me.  But if you prefer any other name,
just name it :)

+    if (variable->mode() == Variable::CONST) {
+      BAILOUT("reference to const context slot");
+    }
+    Slot* slot = variable->AsSlot();
+    CompilationInfo* info = graph()->info();
+    int context_chain_length = info->function()->scope()->
+        ContextChainLength(slot->var()->scope());
+    ASSERT(context_chain_length >= 0);
+    // TODO(antonm): if slot's value is not modified by closures, instead
+    // of reading it out of context, we could just embed the value as
+    // a constant.
+    HLoadContextSlot* instr =
+        new HLoadContextSlot(context_chain_length, slot->index());
+    ast_context()->ReturnInstruction(instr, expr->id());
   } else if (variable->is_global()) {
     LookupResult lookup;
     LookupGlobalPropertyCell(variable, &lookup, false);
@@ -3444,6 +3460,9 @@ void HGraphBuilder::VisitAssignment(Assignment* expr) {
     if (proxy->IsArguments()) BAILOUT("assignment to arguments");
 
     // Handle the assignment.
+    if (var->AsSlot() != NULL && var->AsSlot()->type() == Slot::CONTEXT) {

[Kevin Millikin (Chromium), 2010/12/20 12:05:44]

Instead of an early bailout, it would be better if this code should be
structured like the code in VisitVariableProxy, explicitly handling the cases:

if (/* global */) ...
else if (/* stack-allocated */) ...
else /* the rest, bailout */

[antonm, 2010/12/20 20:39:24]

Done.

+      BAILOUT("context slot assignment");
+    }
     if (var->is_global()) {
       VISIT_FOR_VALUE(expr->value());
       HandleGlobalVariableAssignment(var,