[crankshaft] Reland "Only exclude explicit 'arguments' (and 'this') from liveness analysis."

Reland of https://codereview.chromium.org/2026173003 (reverted by
https://codereview.chromium.org/2029563002).

Additionally, we need to record environment markers even for the
case of a.length, where a is aliased arguments (which crankshaft
optimizes to constant for the inlined case or to HArgumentsLength
when not inlined).

BUG=chromium:612146

Review-Url: https://codereview.chromium.org/2028243002
Cr-Commit-Position: refs/heads/master@{#36662}

src/crankshaft/hydrogen.cc

@@ -7938,6 +7938,11 @@ bool HOptimizedGraphBuilder::TryArgumentsAccess(Property* expr) {
       return false;
     }
 
+    // Make sure we visit the arguments object so that the liveness analysis
+    // still records the access.
+    CHECK_ALIVE_OR_RETURN(VisitForValue(expr->obj(), ARGUMENTS_ALLOWED), true);
+    Drop(1);
+
     if (function_state()->outer() == NULL) {
       HInstruction* elements = Add<HArgumentsElements>(false);
       result = New<HArgumentsLength>(elements);

src/crankshaft/hydrogen.h

@@ -2362,21 +2362,19 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   void Bind(Variable* var, HValue* value) { environment()->Bind(var, value); }
   bool IsEligibleForEnvironmentLivenessAnalysis(Variable* var,
                                                 int index,
-                                                HValue* value,
                                                 HEnvironment* env) {
     if (!FLAG_analyze_environment_liveness) return false;
     // |this| and |arguments| are always live; zapping parameters isn't
     // safe because function.arguments can inspect them at any time.
     return !var->is_this() &&
            !var->is_arguments() &&
-           !value->IsArgumentsObject() &&
            env->is_local_index(index);
   }
   void BindIfLive(Variable* var, HValue* value) {
     HEnvironment* env = environment();
     int index = env->IndexFor(var);
     env->Bind(index, value);
-    if (IsEligibleForEnvironmentLivenessAnalysis(var, index, value, env)) {
+    if (IsEligibleForEnvironmentLivenessAnalysis(var, index, env)) {
       HEnvironmentMarker* bind =
           Add<HEnvironmentMarker>(HEnvironmentMarker::BIND, index);
       USE(bind);
@@ -2388,8 +2386,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   HValue* LookupAndMakeLive(Variable* var) {
     HEnvironment* env = environment();
     int index = env->IndexFor(var);
-    HValue* value = env->Lookup(index);
-    if (IsEligibleForEnvironmentLivenessAnalysis(var, index, value, env)) {
+    if (IsEligibleForEnvironmentLivenessAnalysis(var, index, env)) {
       HEnvironmentMarker* lookup =
           Add<HEnvironmentMarker>(HEnvironmentMarker::LOOKUP, index);
       USE(lookup);
@@ -2397,7 +2394,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
       lookup->set_closure(env->closure());
 #endif
     }
-    return value;
+    return env->Lookup(index);
   }
 
   // The value of the arguments object is allowed in some but not most value

test/mjsunit/regress/regress-612146.js

+// Copyright 2016 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.
+
+// Flags: --allow-natives-syntax
+
+function f() {
+  var arguments_ = arguments;
+  if (undefined) {
+    while (true) {
+      arguments_[0];
+    }
+  } else {
+    %DeoptimizeNow();
+    return arguments_[0];
+  }
+};
+
+f(0);
+f(0);
+%OptimizeFunctionOnNextCall(f);
+assertEquals(1, f(1));
+
+function g() {
+    var a = arguments;
+    %DeoptimizeNow();
+    return a.length;
+}
+
+g(1);
+g(1);
+%OptimizeFunctionOnNextCall(g);
+assertEquals(1, g(1));