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

src/compiler/ast-graph-builder.cc

Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc
index 68cf58905f87e1f62e0d2186409657ba29861d39..b6380412930000df779855ae49ddefd609f1f2af 100644
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -787,6 +787,10 @@ AstGraphBuilder::Environment::CopyAsUnreachable() {
   return env;
 }
 
+AstGraphBuilder::Environment* AstGraphBuilder::Environment::CopyForOsrEntry() {
+  return new (zone())
+      Environment(this, builder_->liveness_analyzer()->NewBlock());
+}
 
 AstGraphBuilder::Environment*
 AstGraphBuilder::Environment::CopyAndShareLiveness() {
@@ -801,8 +805,13 @@ AstGraphBuilder::Environment::CopyAndShareLiveness() {
 
 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;
 }
 
 
@@ -4182,9 +4191,42 @@ void AstGraphBuilder::Environment::Merge(Environment* other) {
   }
 }
 
+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();
+
+  // 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,
-                                                  bool is_osr) {
+void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned) {
   int size = static_cast<int>(values()->size());
 
   Node* control = builder_->NewLoop();
@@ -4219,40 +4261,6 @@ void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned,
       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);
-    }
-  }
 }