[turbofan] Refactor the OSR environment merging to create an explicit environment.

src/compiler/ast-graph-builder.cc

 // Copyright 2014 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/compiler/ast-graph-builder.h"
 
 #include "src/ast/compile-time-value.h"
 #include "src/ast/scopes.h"
 #include "src/compilation-info.h"
 #include "src/compiler.h"
@@ skipping 769 matching lines @@
 }
 
 
 AstGraphBuilder::Environment*
 AstGraphBuilder::Environment::CopyAsUnreachable() {
   Environment* env = new (zone()) Environment(this, nullptr);
   env->MarkAsUnreachable();
   return env;
 }
 
+AstGraphBuilder::Environment* AstGraphBuilder::Environment::CopyForOsrEntry() {
+  return new (zone())
+      Environment(this, builder_->liveness_analyzer()->NewBlock());
+}
 
 AstGraphBuilder::Environment*
 AstGraphBuilder::Environment::CopyAndShareLiveness() {
   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();
+  PrepareForLoop(assigned);
+  Environment* loop = CopyAndShareLiveness();
+  if (is_osr) {
+    // Create and merge the OSR entry if necessary.
+    loop->Merge(PrepareOsrEntry());
+  }
+  return loop;
 }
 
 
 void AstGraphBuilder::Environment::UpdateStateValues(Node** state_values,
                                                      int offset, int count) {
   bool should_update = false;
   Node** env_values = (count == 0) ? nullptr : &values()->at(offset);
   if (*state_values == nullptr || (*state_values)->InputCount() != count) {
     should_update = true;
   } else {
@@ skipping 3359 matching lines @@
   // potentially extending an existing Phi node if possible.
   for (int i = 0; i < static_cast<int>(values_.size()); ++i) {
     values_[i] = builder_->MergeValue(values_[i], other->values_[i], control);
   }
   for (int i = 0; i < static_cast<int>(contexts_.size()); ++i) {
     contexts_[i] =
         builder_->MergeValue(contexts_[i], other->contexts_[i], control);
   }
 }
 
+AstGraphBuilder::Environment* AstGraphBuilder::Environment::PrepareOsrEntry() {

[Michael Starzinger, 2016/09/29 13:14:34]

As discussed offline: This function creates a copy of {this} and then only
mutates the copy. We could enforce this inteded behavior by instead making this
function only work on {this} and invoking it as follows ...

Environmet* osr_entry = CopyForOsrEntry();
osr_entry->PrepareForOsrEntry();
loop->Merge(osr_entry);

This method would then return {void} similar to the existing {PrepareForLoop}
method. IMHO that would keep things uniform. WDYT?

[Jarin, 2016/09/29 13:40:25]

Done. Thanks!

+  int size = static_cast<int>(values()->size());
+  Graph* graph = builder_->graph();
 
-void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned,
-                                                  bool is_osr) {
+  // Create the OSR environment.
+  Environment* osr_env = CopyForOsrEntry();
+  // Set the control and effect to the OSR loop entry.
+  Node* osr_loop_entry = graph->NewNode(builder_->common()->OsrLoopEntry(),
+                                        graph->start(), graph->start());
+  osr_env->UpdateControlDependency(osr_loop_entry);
+  osr_env->UpdateEffectDependency(osr_loop_entry);
+  // Set OSR values.
+  for (int i = 0; i < size; ++i) {
+    osr_env->values()->at(i) =
+        graph->NewNode(builder_->common()->OsrValue(i), osr_loop_entry);
+  }
+
+  // Set the contexts.
+  // The innermost context is the OSR value, and the outer contexts are
+  // reconstructed by dynamically walking up the context chain.
+  Node* osr_context = nullptr;
+  const Operator* op =
+      builder_->javascript()->LoadContext(0, Context::PREVIOUS_INDEX, true);
+  const Operator* op_inner =
+      builder_->common()->OsrValue(Linkage::kOsrContextSpillSlotIndex);
+  int last = static_cast<int>(contexts()->size() - 1);
+  for (int i = last; i >= 0; i--) {
+    osr_context = (i == last) ? graph->NewNode(op_inner, osr_loop_entry)
+                              : graph->NewNode(op, osr_context, osr_context,
+                                               osr_loop_entry);
+    osr_env->contexts()->at(i) = osr_context;
+  }
+  return osr_env;
+}
+
+void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned) {
   int size = static_cast<int>(values()->size());
 
   Node* control = builder_->NewLoop();
   if (assigned == nullptr) {
     // Assume that everything is updated in the loop.
     for (int i = 0; i < size; ++i) {
       values()->at(i) = builder_->NewPhi(1, values()->at(i), control);
     }
   } else {
     // Only build phis for those locals assigned in this loop.
@@ skipping 14 matching lines @@
     builder_->exit_controls_.push_back(terminate);
   }
 
   if (builder_->info()->is_osr()) {
     // Introduce phis for all context values in the case of an OSR graph.
     for (size_t i = 0; i < contexts()->size(); ++i) {
       Node* context = contexts()->at(i);
       contexts()->at(i) = builder_->NewPhi(1, context, control);
     }
   }
-
-  if (is_osr) {
-    // Merge OSR values as inputs to the phis of the loop.
-    Graph* graph = builder_->graph();
-    Node* osr_loop_entry = builder_->graph()->NewNode(
-        builder_->common()->OsrLoopEntry(), graph->start(), graph->start());
-
-    builder_->MergeControl(control, osr_loop_entry);
-    builder_->MergeEffect(effect, osr_loop_entry, control);
-
-    for (int i = 0; i < size; ++i) {
-      Node* value = values()->at(i);
-      Node* osr_value =
-          graph->NewNode(builder_->common()->OsrValue(i), osr_loop_entry);
-      values()->at(i) = builder_->MergeValue(value, osr_value, control);
-    }
-
-    // Rename all the contexts in the environment.
-    // The innermost context is the OSR value, and the outer contexts are
-    // reconstructed by dynamically walking up the context chain.
-    Node* osr_context = nullptr;
-    const Operator* op =
-        builder_->javascript()->LoadContext(0, Context::PREVIOUS_INDEX, true);
-    const Operator* op_inner =
-        builder_->common()->OsrValue(Linkage::kOsrContextSpillSlotIndex);
-    int last = static_cast<int>(contexts()->size() - 1);
-    for (int i = last; i >= 0; i--) {
-      Node* context = contexts()->at(i);
-      osr_context = (i == last) ? graph->NewNode(op_inner, osr_loop_entry)
-                                : graph->NewNode(op, osr_context, osr_context,
-                                                 osr_loop_entry);
-      contexts()->at(i) = builder_->MergeValue(context, osr_context, control);
-    }
-  }
 }
 
 
 Node* AstGraphBuilder::NewPhi(int count, Node* input, Node* control) {
   const Operator* phi_op = common()->Phi(MachineRepresentation::kTagged, count);
   Node** buffer = EnsureInputBufferSize(count + 1);
   MemsetPointer(buffer, input, count);
   buffer[count] = control;
   return graph()->NewNode(phi_op, count + 1, buffer, true);
 }
@@ skipping 58 matching lines @@
     // Phi does not exist yet, introduce one.
     value = NewPhi(inputs, value, control);
     value->ReplaceInput(inputs - 1, other);
   }
   return value;
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8