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

src/crankshaft/hydrogen.cc

Index: src/crankshaft/hydrogen.cc
diff --git a/src/crankshaft/hydrogen.cc b/src/crankshaft/hydrogen.cc
index b93db2478e0af3db40d2cf1374040006b1dd1abd..e6c913486ae24361859b3f1e5322fcfaefcb94a4 100644
--- a/src/crankshaft/hydrogen.cc
+++ b/src/crankshaft/hydrogen.cc
@@ -6934,8 +6934,6 @@ void HOptimizedGraphBuilder::VisitAssignment(Assignment* expr) {
       }
     }
 
-    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:
@@ -9314,8 +9312,6 @@ bool HOptimizedGraphBuilder::TryIndirectCall(Call* expr) {
     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);
@@ -9335,6 +9331,10 @@ void HOptimizedGraphBuilder::BuildFunctionApply(Call* expr) {
   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);
 
@@ -9580,8 +9580,9 @@ bool HOptimizedGraphBuilder::CanBeFunctionApplyArguments(Call* expr) {
   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;
 }