Commit 8b0a6dd6 authored by machenbach's avatar machenbach Committed by Commit bot

Revert of [crankshaft] Only exclude explicit 'arguments' (and 'this') from...

Revert of [crankshaft] Only exclude explicit 'arguments' (and 'this') from liveness analysis. (patchset #2 id:20001 of https://codereview.chromium.org/2026173003/ )

Reason for revert:
Triggers crashes on the deopt fuzzer:
https://build.chromium.org/p/client.v8/builders/V8%20Deopt%20Fuzzer/builds/10608

Repro:
out/Release/d8 --test --random-seed=849179141 --deopt-every-n-times 149 --nohard-abort --nodead-code-elimination --nofold-constants --noconcurrent-recompilation test/webkit/resources/standalone-pre.js test/webkit/dfg-arguments-mixed-alias.js test/webkit/resources/standalone-post.js

Original issue's description:
> [crankshaft] Only exclude explicit 'arguments' (and 'this') from liveness analysis.
>
> Currently, we do not emit EnvironmentMarkers if the hydrogen value
> in the environment is arguments object. As the hydrogen value can change
> for local variables, we emit only some environment markers. That can
> cause environment liveness analysis to mark part of live range as live
> and part as dead. The zapping phase then only inserts zaps in
> live->dead transitions, potentially zapping a live value.
>
> With this CL, we only emit EnvironmentMarkers for 'this' and
> 'arguments' local variables, disregarding the hydrogen value.
>
> BUG=chromium:612146
> LOG=n
>
> Committed: https://crrev.com/1428fbe224dc2df0cb6f59e4959430f7aa614064
> Cr-Commit-Position: refs/heads/master@{#36641}

TBR=jkummerow@chromium.org,jarin@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:612146

Review-Url: https://codereview.chromium.org/2029563002
Cr-Commit-Position: refs/heads/master@{#36644}
parent dc78e0d4
...@@ -2362,19 +2362,21 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { ...@@ -2362,19 +2362,21 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
void Bind(Variable* var, HValue* value) { environment()->Bind(var, value); } void Bind(Variable* var, HValue* value) { environment()->Bind(var, value); }
bool IsEligibleForEnvironmentLivenessAnalysis(Variable* var, bool IsEligibleForEnvironmentLivenessAnalysis(Variable* var,
int index, int index,
HValue* value,
HEnvironment* env) { HEnvironment* env) {
if (!FLAG_analyze_environment_liveness) return false; if (!FLAG_analyze_environment_liveness) return false;
// |this| and |arguments| are always live; zapping parameters isn't // |this| and |arguments| are always live; zapping parameters isn't
// safe because function.arguments can inspect them at any time. // safe because function.arguments can inspect them at any time.
return !var->is_this() && return !var->is_this() &&
!var->is_arguments() && !var->is_arguments() &&
!value->IsArgumentsObject() &&
env->is_local_index(index); env->is_local_index(index);
} }
void BindIfLive(Variable* var, HValue* value) { void BindIfLive(Variable* var, HValue* value) {
HEnvironment* env = environment(); HEnvironment* env = environment();
int index = env->IndexFor(var); int index = env->IndexFor(var);
env->Bind(index, value); env->Bind(index, value);
if (IsEligibleForEnvironmentLivenessAnalysis(var, index, env)) { if (IsEligibleForEnvironmentLivenessAnalysis(var, index, value, env)) {
HEnvironmentMarker* bind = HEnvironmentMarker* bind =
Add<HEnvironmentMarker>(HEnvironmentMarker::BIND, index); Add<HEnvironmentMarker>(HEnvironmentMarker::BIND, index);
USE(bind); USE(bind);
...@@ -2386,7 +2388,8 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { ...@@ -2386,7 +2388,8 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
HValue* LookupAndMakeLive(Variable* var) { HValue* LookupAndMakeLive(Variable* var) {
HEnvironment* env = environment(); HEnvironment* env = environment();
int index = env->IndexFor(var); int index = env->IndexFor(var);
if (IsEligibleForEnvironmentLivenessAnalysis(var, index, env)) { HValue* value = env->Lookup(index);
if (IsEligibleForEnvironmentLivenessAnalysis(var, index, value, env)) {
HEnvironmentMarker* lookup = HEnvironmentMarker* lookup =
Add<HEnvironmentMarker>(HEnvironmentMarker::LOOKUP, index); Add<HEnvironmentMarker>(HEnvironmentMarker::LOOKUP, index);
USE(lookup); USE(lookup);
...@@ -2394,7 +2397,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { ...@@ -2394,7 +2397,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
lookup->set_closure(env->closure()); lookup->set_closure(env->closure());
#endif #endif
} }
return env->Lookup(index); return value;
} }
// The value of the arguments object is allowed in some but not most value // The value of the arguments object is allowed in some but not most value
......
// 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));
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment