Remove ARGUMENTS_VARIABLE and fix crankshaft to properly detect the arguments object and keep it al…

src/crankshaft/hydrogen.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/crankshaft/hydrogen.h"
 
 #include <memory>
 #include <sstream>
 
 #include "src/allocation-site-scopes.h"
@@ skipping 6916 matching lines @@
       if (expr->op() != Token::INIT) {
         if (var->throw_on_const_assignment(function_language_mode())) {
           return Bailout(kNonInitializerAssignmentToConst);
         } else {
           CHECK_ALIVE(VisitForValue(expr->value()));
           return ast_context()->ReturnValue(Pop());
         }
       }
     }
 
-    if (var->is_arguments()) return Bailout(kAssignmentToArguments);

[adamk, 2016/09/22 20:56:29]

I think this BailoutReason can be removed as well.

[Toon Verwaest, 2016/09/23 09:13:02]

Done.

-
     // Handle the assignment.
     switch (var->location()) {
       case VariableLocation::UNALLOCATED:
         CHECK_ALIVE(VisitForValue(expr->value()));
         HandleGlobalVariableAssignment(var, Top(), expr->AssignmentSlot(),
                                        expr->AssignmentId());
         return ast_context()->ReturnValue(Pop());
 
       case VariableLocation::PARAMETER:
       case VariableLocation::LOCAL: {
@@ skipping 2358 matching lines @@
 
   switch (expr->target()->shared()->builtin_function_id()) {
     case kFunctionCall: {
       if (expr->arguments()->length() == 0) return false;
       BuildFunctionCall(expr);
       return true;
     }
     case kFunctionApply: {
       // For .apply, only the pattern f.apply(receiver, arguments)
       // is supported.
-      if (current_info()->scope()->arguments() == NULL) return false;

[adamk, 2016/09/22 20:56:29]

Was this check just redundant?

[Toon Verwaest, 2016/09/23 09:13:02]

Yes, it must be. scope()->arguments() exists if there's a reference; and .apply
inlining only works if the right side is arguments. There's no way to get
arguments without having a reference somewhere. You might have noticed I
replaced it with a dcheck in the case we're going to return true for "can inline
.apply"

-
       if (!CanBeFunctionApplyArguments(expr)) return false;
 
       BuildFunctionApply(expr);
       return true;
     }
     default: { return false; }
   }
   UNREACHABLE();
 }
 
 
 // f.apply(...)
 void HOptimizedGraphBuilder::BuildFunctionApply(Call* expr) {
   ZoneList<Expression*>* args = expr->arguments();
   CHECK_ALIVE(VisitForValue(args->at(0)));
   HValue* receiver = Pop();  // receiver
   HValue* function = Pop();  // f
   Drop(1);  // apply
 
+  // Make sure the arguments object is live.
+  VariableProxy* arg_two = args->at(1)->AsVariableProxy();
+  LookupAndMakeLive(arg_two->var());
+
   Handle<Map> function_map = expr->GetReceiverTypes()->first();
   HValue* checked_function = AddCheckMap(function, function_map);
 
   if (function_state()->outer() == NULL) {
     TailCallMode syntactic_tail_call_mode = expr->tail_call_mode();
     TailCallMode tail_call_mode =
         function_state()->ComputeTailCallMode(syntactic_tail_call_mode);
 
     HInstruction* elements = Add<HArgumentsElements>(false);
     HInstruction* length = Add<HArgumentsLength>(elements);
@@ skipping 225 matching lines @@
 
   return true;
 }
 
 
 bool HOptimizedGraphBuilder::CanBeFunctionApplyArguments(Call* expr) {
   ZoneList<Expression*>* args = expr->arguments();
   if (args->length() != 2) return false;
   VariableProxy* arg_two = args->at(1)->AsVariableProxy();
   if (arg_two == NULL || !arg_two->var()->IsStackAllocated()) return false;
-  HValue* arg_two_value = LookupAndMakeLive(arg_two->var());
+  HValue* arg_two_value = environment()->Lookup(arg_two->var());
   if (!arg_two_value->CheckFlag(HValue::kIsArguments)) return false;
+  DCHECK_NOT_NULL(current_info()->scope()->arguments());
   return true;
 }
 
 
 void HOptimizedGraphBuilder::VisitCall(Call* expr) {
   DCHECK(!HasStackOverflow());
   DCHECK(current_block() != NULL);
   DCHECK(current_block()->HasPredecessor());
   if (!top_info()->is_tracking_positions()) SetSourcePosition(expr->position());
   Expression* callee = expr->expression();
@@ skipping 3690 matching lines @@
     isolate()->GetHTracer()->TraceHydrogen(name(), graph_);
   }
 
 #ifdef DEBUG
   graph_->Verify(false);  // No full verify.
 #endif
 }
 
 }  // namespace internal
 }  // namespace v8