[hydrogen] don't bailout assignments to consts

src/hydrogen.cc

 // 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
@@ skipping 3267 matching lines @@
       if (value == graph()->GetConstantHole()) {
         ASSERT(variable->mode() == CONST ||
                variable->mode() == CONST_HARMONY ||
                variable->mode() == LET);
         return Bailout("reference to uninitialized variable");
       }
       return ast_context()->ReturnValue(value);
     }
 
     case Variable::CONTEXT: {
-      if (variable->mode() == CONST) {
-        return Bailout("reference to const context slot");
-      }
       HValue* context = BuildContextChainWalk(variable);
       HLoadContextSlot* instr = new(zone()) HLoadContextSlot(context, variable);
       return ast_context()->ReturnInstruction(instr, expr->id());
     }
 
     case Variable::LOOKUP:
       return Bailout("reference to a variable which requires dynamic lookup");
   }
 }
 
@@ skipping 497 matching lines @@
   VariableProxy* proxy = target->AsVariableProxy();
   Property* prop = target->AsProperty();
   ASSERT(proxy == NULL || prop == NULL);
 
   // We have a second position recorded in the FullCodeGenerator to have
   // type feedback for the binary operation.
   BinaryOperation* operation = expr->binary_operation();
 
   if (proxy != NULL) {
     Variable* var = proxy->var();
-    if (var->mode() == CONST || var->mode() == LET)  {
+    if (var->mode() == LET)  {
       return Bailout("unsupported let or const compound assignment");
     }
 
     CHECK_ALIVE(VisitForValue(operation));
 
     switch (var->location()) {
       case Variable::UNALLOCATED:
         HandleGlobalVariableAssignment(var,
                                        Top(),
                                        expr->position(),
                                        expr->AssignmentId());
         break;
 
       case Variable::PARAMETER:
       case Variable::LOCAL:
         Bind(var, Top());

[Steven, 2011/12/12 13:12:44]

mjsunit/compiler/regress-const.js fails because of a compound assignment to a
stack allocated CONST variable. You should also skip the assignment here.

         break;
 
       case Variable::CONTEXT: {
         // Bail out if we try to mutate a parameter value in a function
         // using the arguments object.  We do not (yet) correctly handle the
         // arguments property of the function.
         if (info()->scope()->arguments() != NULL) {
           // Parameters will be allocated to context slots.  We have no
           // direct way to detect that the variable is a parameter so we do
           // a linear search of the parameter variables.
           int count = info()->scope()->num_parameters();
           for (int i = 0; i < count; ++i) {
             if (var == info()->scope()->parameter(i)) {
               Bailout(
                   "assignment to parameter, function uses arguments object");
             }
           }
         }
 
+        HStoreContextSlot::Mode mode;
+
+        switch (var->mode()) {
+          case LET:
+          case CONST_HARMONY:

[Steven, 2011/12/12 13:12:44]

You can put an UNREACHABLE() in the CONST_HARMONY case, because we statically
check for this case and throw an early syntax error.

+            mode = HStoreContextSlot::kCheckDeoptimize;
+            break;
+          case CONST:
+            return ast_context()->ReturnValue(Pop());
+          default:
+            mode = HStoreContextSlot::kNoCheck;
+        }
+
         HValue* context = BuildContextChainWalk(var);
-        HStoreContextSlot::Mode mode =
-            (var->mode() == LET || var->mode() == CONST_HARMONY)
-            ? HStoreContextSlot::kAssignCheck : HStoreContextSlot::kAssign;
         HStoreContextSlot* instr =
             new(zone()) HStoreContextSlot(context, var->index(), mode, Top());
         AddInstruction(instr);
         if (instr->HasObservableSideEffects()) {
           AddSimulate(expr->AssignmentId());
         }
         break;
       }
 
       case Variable::LOOKUP:
@@ skipping 90 matching lines @@
 
   if (expr->is_compound()) {
     HandleCompoundAssignment(expr);
     return;
   }
 
   if (prop != NULL) {
     HandlePropertyAssignment(expr);
   } else if (proxy != NULL) {
     Variable* var = proxy->var();
+
     if (var->mode() == CONST) {
       if (expr->op() != Token::INIT_CONST) {
-        return Bailout("non-initializer assignment to const");
+        CHECK_ALIVE(VisitForValue(expr->value()));
+        return ast_context()->ReturnValue(Pop());
       }
-      if (!var->IsStackAllocated()) {
-        return Bailout("assignment to const context slot");
+
+      if (var->IsStackAllocated()) {
+        // 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);
+        AddInstruction(new HUseConst(old_value));
       }
-      // 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);
-      AddInstruction(new HUseConst(old_value));
     } else if (var->mode() == CONST_HARMONY) {
       if (expr->op() != Token::INIT_CONST_HARMONY) {
         return Bailout("non-initializer assignment to const");
       }
     }
 
     if (proxy->IsArguments()) return Bailout("assignment to arguments");
 
     // Handle the assignment.
     switch (var->location()) {
@@ skipping 18 matching lines @@
         // We do not allow the arguments object to occur in a context where it
         // may escape, but assignments to stack-allocated locals are
         // permitted.
         CHECK_ALIVE(VisitForValue(expr->value(), ARGUMENTS_ALLOWED));
         HValue* value = Pop();
         Bind(var, value);
         return ast_context()->ReturnValue(value);
       }
 
       case Variable::CONTEXT: {
-        ASSERT(var->mode() != CONST);
         // Bail out if we try to mutate a parameter value in a function using
         // the arguments object.  We do not (yet) correctly handle the
         // arguments property of the function.
         if (info()->scope()->arguments() != NULL) {
           // Parameters will rewrite to context slots.  We have no direct way
           // to detect that the variable is a parameter.
           int count = info()->scope()->num_parameters();
           for (int i = 0; i < count; ++i) {
             if (var == info()->scope()->parameter(i)) {
               return Bailout("assignment to parameter in arguments object");
             }
           }
         }
 
         CHECK_ALIVE(VisitForValue(expr->value()));
-        HValue* context = BuildContextChainWalk(var);
         HStoreContextSlot::Mode mode;
         if (expr->op() == Token::ASSIGN) {
-          mode = (var->mode() == LET || var->mode() == CONST_HARMONY)
-              ? HStoreContextSlot::kAssignCheck : HStoreContextSlot::kAssign;
+          switch (var->mode()) {
+            case LET:
+            case CONST_HARMONY:

[Steven, 2011/12/12 13:12:44]

Here also CONST_HARMONY should be UNREACHABLE().

+              mode = HStoreContextSlot::kCheckDeoptimize;
+              break;
+            case CONST:
+              return ast_context()->ReturnValue(Pop());
+            default:
+              mode = HStoreContextSlot::kNoCheck;
+          }
+        } else if (expr->op() == Token::INIT_VAR ||
+                   expr->op() == Token::INIT_LET) {
+          mode = HStoreContextSlot::kNoCheck;
         } else {
-          ASSERT(expr->op() == Token::INIT_VAR ||
-                 expr->op() == Token::INIT_LET ||
-                 expr->op() == Token::INIT_CONST_HARMONY);
-          mode = HStoreContextSlot::kAssign;
+          ASSERT(expr->op() == Token::INIT_CONST_HARMONY ||

[Steven, 2011/12/12 13:12:44]

Here CONST_HARMONY can be moved to the HStoreContextSlot::kNoCheck group,
because in contrast to CONST repeated initializations (for example in a loop)
are not possible with CONST_HARMONY.

+                 expr->op() == Token::INIT_CONST);
+
+          mode = HStoreContextSlot::kCheckIgnoreAssignment;
         }
+
+        HValue* context = BuildContextChainWalk(var);
         HStoreContextSlot* instr = new(zone()) HStoreContextSlot(
             context, var->index(), mode, Top());
         AddInstruction(instr);
         if (instr->HasObservableSideEffects()) {
           AddSimulate(expr->AssignmentId());
         }
         return ast_context()->ReturnValue(Pop());
       }
 
       case Variable::LOOKUP:
@@ skipping 1550 matching lines @@
   // element for postfix operations in a non-effect context.  The return
   // value is ToNumber(input).
   bool returns_original_input =
       expr->is_postfix() && !ast_context()->IsEffect();
   HValue* input = NULL;  // ToNumber(original_input).
   HValue* after = NULL;  // The result after incrementing or decrementing.
 
   if (proxy != NULL) {
     Variable* var = proxy->var();
     if (var->mode() == CONST)  {
       return Bailout("unsupported count operation with const");

[Steven, 2011/12/12 13:12:44]

It should be trivial to also remove this bailout and do the same thing as in the
CompoundAssignments and ignore the assignment to CONST. Could you also add
simple test cases, that test that crankshafted CompoundAssignments and
CountOperations have the expected behaviour for context allocated CONST
bindings?

     }
     // Argument of the count operation is a variable, not a property.
     ASSERT(prop == NULL);
     CHECK_ALIVE(VisitForValue(target));
 
     after = BuildIncrement(returns_original_input, expr);
     input = returns_original_input ? Top() : Pop();
     Push(after);
 
     switch (var->location()) {
@@ skipping 21 matching lines @@
           for (int i = 0; i < count; ++i) {
             if (var == info()->scope()->parameter(i)) {
               return Bailout("assignment to parameter in arguments object");
             }
           }
         }
 
         HValue* context = BuildContextChainWalk(var);
         HStoreContextSlot::Mode mode =
             (var->mode() == LET || var->mode() == CONST_HARMONY)
-            ? HStoreContextSlot::kAssignCheck : HStoreContextSlot::kAssign;
+            ? HStoreContextSlot::kCheckDeoptimize : HStoreContextSlot::kNoCheck;
         HStoreContextSlot* instr =
             new(zone()) HStoreContextSlot(context, var->index(), mode, after);
         AddInstruction(instr);
         if (instr->HasObservableSideEffects()) {
           AddSimulate(expr->AssignmentId());
         }
         break;
       }
 
       case Variable::LOOKUP:
@@ skipping 587 matching lines @@
         HValue* value = NULL;
         if (function != NULL) {
           VisitForValue(function);
           value = Pop();
         } else {
           value = graph()->GetConstantHole();
         }
         if (var->IsContextSlot()) {
           HValue* context = environment()->LookupContext();
           HStoreContextSlot* store = new HStoreContextSlot(
-              context, var->index(), HStoreContextSlot::kAssign, value);
+              context, var->index(), HStoreContextSlot::kNoCheck, value);
           AddInstruction(store);
           if (store->HasObservableSideEffects()) AddSimulate(proxy->id());
         } else {
           environment()->Bind(var, value);
         }
       }
       break;
     case Variable::LOOKUP:
       return Bailout("unsupported lookup slot in declaration");
   }
@@ skipping 1001 matching lines @@
     }
   }
 
 #ifdef DEBUG
   if (graph_ != NULL) graph_->Verify(false);  // No full verify.
   if (allocator_ != NULL) allocator_->Verify();
 #endif
 }
 
 } }  // namespace v8::internal