Simple support for const variables in Crankshaft....

src/hydrogen.cc

Index: src/hydrogen.cc
===================================================================
--- src/hydrogen.cc	(revision 6080)
+++ src/hydrogen.cc	(working copy)
@@ -502,6 +502,11 @@
 }
 
 
+HConstant* HGraph::GetConstantHole() {
+  return GetConstant(&constant_hole_, Heap::the_hole_value());
+}
+
+
 void HSubgraph::AppendOptional(HSubgraph* graph,
                                bool on_true_branch,
                                HValue* boolean_value) {
@@ -891,6 +896,10 @@
       phi_list_->Add(phi);
       // We don't support phi uses of arguments for now.
       if (phi->CheckFlag(HValue::kIsArguments)) return false;
+      // Check for the hole value (from an uninitialized const).
+      for (int k = 0; k < phi->OperandCount(); k++) {
+        if (phi->OperandAt(k) == GetConstantHole()) return false;
+      }
     }
   }
   return true;
@@ -2220,7 +2229,7 @@
   graph_->AssignDominators();
   graph_->EliminateRedundantPhis();
   if (!graph_->CollectPhis()) {
-    Bailout("Phi-use of arguments object");
+    Bailout("Unsupported phi-use");
     return NULL;
   }
 
@@ -2329,6 +2338,8 @@
   // We can't handle heap-allocated locals.
   if (scope->num_heap_slots() > 0) BAILOUT("heap allocated locals");
 
+  if (scope->HasIllegalRedeclaration()) BAILOUT("illegal redeclaration");
+
   HConstant* undefined_constant =
       new HConstant(Factory::undefined_value(), Representation::Tagged());
   AddInstruction(undefined_constant);
@@ -2936,10 +2947,15 @@
   if (variable == NULL) {
     BAILOUT("reference to rewritten variable");
   } else if (variable->IsStackAllocated()) {
-    if (environment()->Lookup(variable)->CheckFlag(HValue::kIsArguments)) {
+    HValue* value = environment()->Lookup(variable);
+    if (value->CheckFlag(HValue::kIsArguments)) {
       BAILOUT("unsupported context for arguments object");
     }
-    ast_context()->ReturnValue(environment()->Lookup(variable));
+    if (variable->mode() == Variable::CONST &&
+        value == graph()->GetConstantHole()) {
+      BAILOUT("unsupported reference to const variable");

[Kevin Millikin (Chromium), 2011/01/04 13:46:33]

ast_context()->ReturnValue(graph()->GetConstantUndefined());

+    }
+    ast_context()->ReturnValue(value);
   } else if (variable->is_global()) {
     LookupResult lookup;
     LookupGlobalPropertyCell(variable, &lookup, false);
@@ -3358,6 +3374,7 @@
   operation->RecordTypeFeedback(oracle());
 
   if (var != NULL) {
+    if (var->mode() == Variable::CONST) BAILOUT("unsupported const assignment");

[Kevin Millikin (Chromium), 2011/01/04 13:46:33]

Instead of statically bailing out, how about visiting the operation and then
guarding the assignment with something like

if (var->mode() != Variable::CONST) ....

     if (!var->is_global() && !var->IsStackAllocated()) {
       BAILOUT("non-stack/non-global in compound assignment");
     }
@@ -3462,6 +3479,17 @@
   }
 
   if (var != NULL) {
+    if (var->mode() == Variable::CONST) {
+      if (expr->op() != Token::INIT_CONST) {
+        BAILOUT("unsupported const assignment");

[Kevin Millikin (Chromium), 2011/01/04 13:46:33]

Instead of statically bailing out for non-INIT_CONST assignments to CONST
variables, you could just ignore them.

+      }
+      // We insert a use of the old value to detect unsupported uses of const
+      // variables (e.g. initialization inside a loop).
+      HValue* old_value = environment()->Lookup(var);

[Kevin Millikin (Chromium), 2011/01/04 13:46:33]

It's a little indirect to add this use and detect it in a postprocessing pass. 
It seems more direct (though a bit more implementation effort) to keep track of
a loop nesting depth and bailout if non-zero.

I'm also not thrilled with InitConst as an instruction.  It's not initializing
anything and it's odd that it refers to the old value and not the new.  It's
more like UseValue or something.

+      AddInstruction(new HInitConst(old_value));
+    }
+
+
     if (proxy->IsArguments()) BAILOUT("assignment to arguments");
 
     // Handle the assignment.
@@ -4534,6 +4562,7 @@
   bool inc = expr->op() == Token::INC;
 
   if (var != NULL) {
+    if (var->mode() == Variable::CONST) BAILOUT("unsupported count operation");

[Kevin Millikin (Chromium), 2011/01/04 13:46:33]

Same comment.

     if (!var->is_global() && !var->IsStackAllocated()) {
       BAILOUT("non-stack/non-global variable in count operation");
     }
@@ -4888,16 +4917,18 @@
 void HGraphBuilder::VisitDeclaration(Declaration* decl) {
   // We allow only declarations that do not require code generation.
   // The following all require code generation: global variables and
-  // functions, variables with slot type LOOKUP, declarations with
-  // mode CONST, and functions.
+  // functions, variables with slot type LOOKUP

[Kevin Millikin (Chromium), 2011/01/04 13:46:33]

Fix the comment so it's still a sentence (add "and" and a period).

   Variable* var = decl->proxy()->var();
   Slot* slot = var->AsSlot();
   if (var->is_global() ||
       (slot != NULL && slot->type() == Slot::LOOKUP) ||
-      decl->mode() == Variable::CONST ||
       decl->fun() != NULL) {
     BAILOUT("unsupported declaration");
   }
+
+  if (var->mode() == Variable::CONST) {

[Kevin Millikin (Chromium), 2011/01/04 13:46:33]

I don't think you should change the test against decl->mode() to one against
var->mode().  Declaration::mode() can only be VAR or CONST, while
Variable::mode() could be all kinds of other stuff (DYNAMIC, DYNAMIC_GLOBAL,
etc).

I'm worried that things will break as we support more stuff if we don't handle
constants more or less like the baseline compiler.

+    environment()->Bind(var, graph()->GetConstantHole());

[Kevin Millikin (Chromium), 2011/01/04 13:46:33]

I'm surprised you don't have to check that the variable is stack allocated here.

+  }
 }