[turbofan] Fix variable liveness control structure creation.

R=bmeurer@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1148133002

Cr-Commit-Position: refs/heads/master@{#28536}

diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc
index 1c98099..0d01a5a 100644 (file)
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -599,7 +599,10 @@ void AstGraphBuilder::CreateGraphBody(bool stack_check) {
 
 
 void AstGraphBuilder::ClearNonLiveSlotsInFrameStates() {
-  if (!FLAG_analyze_environment_liveness) return;
+  if (!FLAG_analyze_environment_liveness ||
+      !info()->is_deoptimization_enabled()) {
+    return;
+  }
 
   NonLiveFrameStateSlotReplacer replacer(
       &state_values_cache_, jsgraph()->UndefinedConstant(),
@@ -660,7 +663,9 @@ AstGraphBuilder::Environment::Environment(AstGraphBuilder* builder,
     : builder_(builder),
       parameters_count_(scope->num_parameters() + 1),
       locals_count_(scope->num_stack_slots()),
-      liveness_block_(builder_->liveness_analyzer()->NewBlock()),
+      liveness_block_(IsLivenessAnalysisEnabled()
+                          ? builder_->liveness_analyzer()->NewBlock()
+                          : nullptr),
       values_(builder_->local_zone()),
       contexts_(builder_->local_zone()),
       control_dependency_(control_dependency),
@@ -696,10 +701,12 @@ AstGraphBuilder::Environment::Environment(AstGraphBuilder* builder,
 }
 
 
-AstGraphBuilder::Environment::Environment(AstGraphBuilder::Environment* copy)
+AstGraphBuilder::Environment::Environment(AstGraphBuilder::Environment* copy,
+                                          LivenessAnalyzerBlock* liveness_block)
     : builder_(copy->builder_),
       parameters_count_(copy->parameters_count_),
       locals_count_(copy->locals_count_),
+      liveness_block_(liveness_block),
       values_(copy->zone()),
       contexts_(copy->zone()),
       control_dependency_(copy->control_dependency_),
@@ -713,14 +720,6 @@ AstGraphBuilder::Environment::Environment(AstGraphBuilder::Environment* copy)
   contexts_.reserve(copy->contexts_.size());
   contexts_.insert(contexts_.begin(), copy->contexts_.begin(),
                    copy->contexts_.end());
-
-  if (FLAG_analyze_environment_liveness) {
-    // Split the liveness blocks.
-    copy->liveness_block_ =
-        builder_->liveness_analyzer()->NewBlock(copy->liveness_block());
-    liveness_block_ =
-        builder_->liveness_analyzer()->NewBlock(copy->liveness_block());
-  }
 }
 
 
@@ -733,7 +732,9 @@ void AstGraphBuilder::Environment::Bind(Variable* variable, Node* node) {
   } else {
     DCHECK(variable->IsStackLocal());
     values()->at(variable->index() + parameters_count_) = node;
-    if (FLAG_analyze_environment_liveness) {
+
+    DCHECK(IsLivenessBlockConsistent());
+    if (liveness_block() != nullptr) {
       liveness_block()->Bind(variable->index());
     }
   }
@@ -748,7 +749,8 @@ Node* AstGraphBuilder::Environment::Lookup(Variable* variable) {
     return values()->at(variable->index() + 1);
   } else {
     DCHECK(variable->IsStackLocal());
-    if (FLAG_analyze_environment_liveness) {
+    DCHECK(IsLivenessBlockConsistent());
+    if (liveness_block() != nullptr) {
       liveness_block()->Lookup(variable->index());
     }
     return values()->at(variable->index() + parameters_count_);
@@ -757,7 +759,8 @@ Node* AstGraphBuilder::Environment::Lookup(Variable* variable) {
 
 
 void AstGraphBuilder::Environment::MarkAllLocalsLive() {
-  if (FLAG_analyze_environment_liveness) {
+  DCHECK(IsLivenessBlockConsistent());
+  if (liveness_block() != nullptr) {
     for (int i = 0; i < locals_count_; i++) {
       liveness_block()->Lookup(i);
     }
@@ -766,15 +769,43 @@ void AstGraphBuilder::Environment::MarkAllLocalsLive() {
 
 
 AstGraphBuilder::Environment*
+AstGraphBuilder::Environment::CopyForConditional() {
+  LivenessAnalyzerBlock* copy_liveness_block = nullptr;
+  if (liveness_block() != nullptr) {
+    copy_liveness_block =
+        builder_->liveness_analyzer()->NewBlock(liveness_block());
+    liveness_block_ = builder_->liveness_analyzer()->NewBlock(liveness_block());
+  }
+  return new (zone()) Environment(this, copy_liveness_block);
+}
+
+
+AstGraphBuilder::Environment*
+AstGraphBuilder::Environment::CopyAsUnreachable() {
+  Environment* env = new (zone()) Environment(this, nullptr);
+  env->MarkAsUnreachable();
+  return env;
+}
+
+
+AstGraphBuilder::Environment*
 AstGraphBuilder::Environment::CopyAndShareLiveness() {
-  Environment* env = new (zone()) Environment(this);
-  if (FLAG_analyze_environment_liveness) {
-    env->liveness_block_ = liveness_block();
+  if (liveness_block() != nullptr) {
+    // Finish the current liveness block before copying.
+    liveness_block_ = builder_->liveness_analyzer()->NewBlock(liveness_block());
   }
+  Environment* env = new (zone()) Environment(this, liveness_block());
   return env;
 }
 
 
+AstGraphBuilder::Environment* AstGraphBuilder::Environment::CopyForLoop(
+    BitVector* assigned, bool is_osr) {
+  PrepareForLoop(assigned, is_osr);
+  return CopyAndShareLiveness();
+}
+
+
 void AstGraphBuilder::Environment::UpdateStateValues(Node** state_values,
                                                      int offset, int count) {
   bool should_update = false;
@@ -822,13 +853,27 @@ Node* AstGraphBuilder::Environment::Checkpoint(
                                   stack_node_, builder()->current_context(),
                                   builder()->GetFunctionClosure(),
                                   builder()->jsgraph()->UndefinedConstant());
-  if (FLAG_analyze_environment_liveness) {
+
+  DCHECK(IsLivenessBlockConsistent());
+  if (liveness_block() != nullptr) {
     liveness_block()->Checkpoint(result);
   }
   return result;
 }
 
 
+bool AstGraphBuilder::Environment::IsLivenessAnalysisEnabled() {
+  return FLAG_analyze_environment_liveness &&
+         builder()->info()->is_deoptimization_enabled();
+}
+
+
+bool AstGraphBuilder::Environment::IsLivenessBlockConsistent() {
+  return (!IsLivenessAnalysisEnabled() || IsMarkedAsUnreachable()) ==
+         (liveness_block() == nullptr);
+}
+
+
 AstGraphBuilder::AstContext::AstContext(AstGraphBuilder* own,
                                         Expression::Context kind)
     : kind_(kind), owner_(own), outer_(own->ast_context()) {
@@ -3505,9 +3550,13 @@ Node* AstGraphBuilder::MakeNode(const Operator* op, int value_input_count,
       }
       // Add implicit exception continuation for throwing nodes.
       if (!result->op()->HasProperty(Operator::kNoThrow) && inside_try_scope) {
+        // Copy the environment for the success continuation.
+        Environment* success_env = environment()->CopyForConditional();
+
         Node* on_exception = graph()->NewNode(common()->IfException(), result);
         environment_->UpdateControlDependency(on_exception);
         execution_control()->ThrowValue(on_exception);
+        set_environment(success_env);
       }
       // Add implicit success continuation for throwing nodes.
       if (!result->op()->HasProperty(Operator::kNoThrow)) {
@@ -3543,21 +3592,23 @@ void AstGraphBuilder::Environment::Merge(Environment* other) {
   if (this->IsMarkedAsUnreachable()) {
     Node* other_control = other->control_dependency_;
     Node* inputs[] = {other_control};
-    liveness_block_ = other->liveness_block_;
     control_dependency_ =
         graph()->NewNode(common()->Merge(1), arraysize(inputs), inputs, true);
     effect_dependency_ = other->effect_dependency_;
     values_ = other->values_;
     contexts_ = other->contexts_;
+    if (IsLivenessAnalysisEnabled()) {
+      liveness_block_ =
+          builder_->liveness_analyzer()->NewBlock(other->liveness_block());
+    }
     return;
   }
 
   // Record the merge for the local variable liveness calculation.
-  // Unfortunately, we have to mirror the logic in the MergeControl method:
-  // connect before merge or loop, or create a new merge otherwise.
-  if (FLAG_analyze_environment_liveness) {
-    if (GetControlDependency()->opcode() != IrOpcode::kLoop &&
-        GetControlDependency()->opcode() != IrOpcode::kMerge) {
+  // For loops, we are connecting a back edge into the existing block;
+  // for merges, we create a new merged block.
+  if (IsLivenessAnalysisEnabled()) {
+    if (GetControlDependency()->opcode() != IrOpcode::kLoop) {
       liveness_block_ =
           builder_->liveness_analyzer()->NewBlock(liveness_block());
     }

diff --git a/src/compiler/ast-graph-builder.h b/src/compiler/ast-graph-builder.h
index ce959b2..c7d604c 100644 (file)
--- a/src/compiler/ast-graph-builder.h
+++ b/src/compiler/ast-graph-builder.h
@@ -459,6 +459,7 @@ class AstGraphBuilder::Environment : public ZoneObject {
   // Mark this environment as being unreachable.
   void MarkAsUnreachable() {
     UpdateControlDependency(builder()->jsgraph()->DeadControl());
+    liveness_block_ = nullptr;
   }
   bool IsMarkedAsUnreachable() {
     return GetControlDependency()->opcode() == IrOpcode::kDead;
@@ -468,20 +469,13 @@ class AstGraphBuilder::Environment : public ZoneObject {
   void Merge(Environment* other);
 
   // Copies this environment at a control-flow split point.
-  Environment* CopyForConditional() { return Copy(); }
+  Environment* CopyForConditional();
 
   // Copies this environment to a potentially unreachable control-flow point.
-  Environment* CopyAsUnreachable() {
-    Environment* env = Copy();
-    env->MarkAsUnreachable();
-    return env;
-  }
+  Environment* CopyAsUnreachable();
 
   // Copies this environment at a loop header control-flow point.
-  Environment* CopyForLoop(BitVector* assigned, bool is_osr = false) {
-    PrepareForLoop(assigned, is_osr);
-    return CopyAndShareLiveness();
-  }
+  Environment* CopyForLoop(BitVector* assigned, bool is_osr = false);
 
  private:
   AstGraphBuilder* builder_;
@@ -496,8 +490,8 @@ class AstGraphBuilder::Environment : public ZoneObject {
   Node* locals_node_;
   Node* stack_node_;
 
-  explicit Environment(Environment* copy);
-  Environment* Copy() { return new (zone()) Environment(this); }
+  explicit Environment(Environment* copy,
+                       LivenessAnalyzerBlock* liveness_block);
   Environment* CopyAndShareLiveness();
   void UpdateStateValues(Node** state_values, int offset, int count);
   void UpdateStateValuesWithCache(Node** state_values, int offset, int count);
@@ -508,6 +502,8 @@ class AstGraphBuilder::Environment : public ZoneObject {
   NodeVector* values() { return &values_; }
   NodeVector* contexts() { return &contexts_; }
   LivenessAnalyzerBlock* liveness_block() { return liveness_block_; }
+  bool IsLivenessAnalysisEnabled();
+  bool IsLivenessBlockConsistent();
 
   // Prepare environment to be used as loop header.
   void PrepareForLoop(BitVector* assigned, bool is_osr = false);

diff --git a/test/mjsunit/compiler/regress-variable-liveness.js b/test/mjsunit/compiler/regress-variable-liveness.js
new file mode 100644 (file)
index 0000000..e18741d
--- /dev/null
+++ b/test/mjsunit/compiler/regress-variable-liveness.js
@@ -0,0 +1,22 @@
+// Copyright 2015 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 foo(x) {
+  %DeoptimizeFunction(run);
+  return x;
+}
+
+function run() {
+  var line = new Array(2);
+  for (var n = 3; n > 0; n = n - 1) {
+    if (n < foo(line.length)) line = new Array(n);
+    line[0] = n;
+  }
+}
+
+assertEquals(void 0, run());
+%OptimizeFunctionOnNextCall(run);
+assertEquals(void 0, run());