[turbofan] Osr value typing + dynamic type checks on entry.

This introduces a new OsrGuard node that is inserted during graph building
to guard the inferred type of the OSR value.

The type of the OSR value is inferred by running the typer before OSR
deconstruction, and then taking the type from the phi that takes the
OSR value. After the deconstruction, we throw the types away.

At the moment we only support the SignedSmall OSR type and we always
pick the tagged representation. Later, we might want to support more
types (such as Number) and pick better representations (int32/float64).

This CL also removes the OSR deconstruction tests because they build
unrealistic graph (no effect chain, no loop termination). I considered
adding the effect chains to the tests, but this would make the tests
even more brittle.

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

src/compiler/ast-graph-builder.cc

@@ -788,8 +788,10 @@ AstGraphBuilder::Environment::CopyAsUnreachable() {
 }
 
 AstGraphBuilder::Environment* AstGraphBuilder::Environment::CopyForOsrEntry() {
-  return new (zone())
-      Environment(this, builder_->liveness_analyzer()->NewBlock());
+  LivenessAnalyzerBlock* copy_block =
+      liveness_block() == nullptr ? nullptr
+                                  : builder_->liveness_analyzer()->NewBlock();
+  return new (zone()) Environment(this, copy_block);
 }
 
 AstGraphBuilder::Environment*
@@ -4202,27 +4204,47 @@ void AstGraphBuilder::Environment::PrepareForOsrEntry() {
                                         graph->start(), graph->start());
   UpdateControlDependency(osr_loop_entry);
   UpdateEffectDependency(osr_loop_entry);
+
   // Set OSR values.
   for (int i = 0; i < size; ++i) {
     values()->at(i) =
         graph->NewNode(builder_->common()->OsrValue(i), osr_loop_entry);
   }
 
-  // Set the contexts.
+  // Set the innermost context.
+  const Operator* op_inner =
+      builder_->common()->OsrValue(Linkage::kOsrContextSpillSlotIndex);
+  contexts()->back() = graph->NewNode(op_inner, osr_loop_entry);
+
+  // Create a checkpoint.
+  Node* frame_state = Checkpoint(builder_->info()->osr_ast_id());
+  Node* checkpoint = graph->NewNode(common()->Checkpoint(), frame_state,
+                                    osr_loop_entry, osr_loop_entry);
+  UpdateEffectDependency(checkpoint);
+
+  // Create the OSR guard nodes.
+  const Operator* guard_op =
+      builder_->common()->OsrGuard(OsrGuardType::kUninitialized);
+  Node* effect = checkpoint;
+  for (int i = 0; i < size; ++i) {
+    values()->at(i) = effect =
+        graph->NewNode(guard_op, values()->at(i), effect, osr_loop_entry);
+  }
+  contexts()->back() = effect =
+      graph->NewNode(guard_op, contexts()->back(), effect, osr_loop_entry);
+
   // 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 =
+  const Operator* load_op =
       builder_->javascript()->LoadContext(0, Context::PREVIOUS_INDEX, true);
-  const Operator* op_inner =
-      builder_->common()->OsrValue(Linkage::kOsrContextSpillSlotIndex);
+  Node* osr_context = effect = contexts()->back();
   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);
+  for (int i = last - 1; i >= 0; i--) {
+    osr_context = effect =
+        graph->NewNode(load_op, osr_context, effect, osr_loop_entry);
     contexts()->at(i) = osr_context;
   }
+  UpdateEffectDependency(effect);
 }
 
 void AstGraphBuilder::Environment::PrepareForLoop(BitVector* assigned) {

src/compiler/bytecode-graph-builder.cc

@@ -438,6 +438,24 @@ void BytecodeGraphBuilder::Environment::PrepareForOsrEntry() {
     if (i >= accumulator_base()) idx = Linkage::kOsrAccumulatorRegisterIndex;
     values()->at(i) = graph()->NewNode(common()->OsrValue(idx), entry);
   }
+
+  BailoutId loop_id(builder_->bytecode_iterator().current_offset());
+  Node* frame_state =
+      Checkpoint(loop_id, OutputFrameStateCombine::Ignore(), false);
+  Node* checkpoint =
+      graph()->NewNode(common()->Checkpoint(), frame_state, entry, entry);
+  UpdateEffectDependency(checkpoint);
+
+  // Create the OSR guard nodes.
+  const Operator* guard_op = common()->OsrGuard(OsrGuardType::kUninitialized);
+  Node* effect = checkpoint;
+  for (int i = 0; i < size; i++) {
+    values()->at(i) = effect =
+        graph()->NewNode(guard_op, values()->at(i), effect, entry);
+  }
+  Node* context = effect = graph()->NewNode(guard_op, Context(), effect, entry);
+  SetContext(context);
+  UpdateEffectDependency(effect);
 }
 
 bool BytecodeGraphBuilder::Environment::StateValuesRequireUpdate(

src/compiler/common-operator.cc

@@ -210,6 +210,31 @@ std::ostream& operator<<(std::ostream& os,
   return os;
 }
 
+int OsrValueIndexOf(Operator const* op) {
+  DCHECK_EQ(IrOpcode::kOsrValue, op->opcode());
+  return OpParameter<int>(op);
+}
+
+size_t hash_value(OsrGuardType type) { return static_cast<size_t>(type); }
+
+std::ostream& operator<<(std::ostream& os, OsrGuardType type) {
+  switch (type) {
+    case OsrGuardType::kUninitialized:
+      return os << "Uninitialized";
+    case OsrGuardType::kSignedSmall:
+      return os << "SignedSmall";
+    case OsrGuardType::kAny:
+      return os << "Any";
+  }
+  UNREACHABLE();
+  return os;
+}
+
+OsrGuardType OsrGuardTypeOf(Operator const* op) {
+  DCHECK_EQ(IrOpcode::kOsrGuard, op->opcode());
+  return OpParameter<OsrGuardType>(op);
+}
+
 #define CACHED_OP_LIST(V)                                                     \
   V(Dead, Operator::kFoldable, 0, 0, 0, 1, 1, 1)                              \
   V(IfTrue, Operator::kKontrol, 0, 0, 1, 0, 0, 1)                             \
@@ -780,7 +805,6 @@ const Operator* CommonOperatorBuilder::Parameter(int index,
       ParameterInfo(index, debug_name));         // parameter info
 }
 
-
 const Operator* CommonOperatorBuilder::OsrValue(int index) {
   return new (zone()) Operator1<int>(                // --
       IrOpcode::kOsrValue, Operator::kNoProperties,  // opcode
@@ -789,6 +813,13 @@ const Operator* CommonOperatorBuilder::OsrValue(int index) {
       index);                                        // parameter
 }
 
+const Operator* CommonOperatorBuilder::OsrGuard(OsrGuardType type) {
+  return new (zone()) Operator1<OsrGuardType>(  // --
+      IrOpcode::kOsrGuard, Operator::kNoThrow,  // opcode
+      "OsrGuard",                               // name
+      1, 1, 1, 1, 1, 0,                         // counts
+      type);                                    // parameter
+}
 
 const Operator* CommonOperatorBuilder::Int32Constant(int32_t value) {
   return new (zone()) Operator1<int32_t>(         // --

src/compiler/common-operator.h

@@ -171,6 +171,13 @@ std::ostream& operator<<(std::ostream& os,
 
 Type* TypeGuardTypeOf(Operator const*) WARN_UNUSED_RESULT;
 
+int OsrValueIndexOf(Operator const*);
+
+enum class OsrGuardType { kUninitialized, kSignedSmall, kAny };
+size_t hash_value(OsrGuardType type);
+std::ostream& operator<<(std::ostream&, OsrGuardType);
+OsrGuardType OsrGuardTypeOf(Operator const*);
+
 // Interface for building common operators that can be used at any level of IR,
 // including JavaScript, mid-level, and low-level.
 class CommonOperatorBuilder final : public ZoneObject {
@@ -202,6 +209,7 @@ class CommonOperatorBuilder final : public ZoneObject {
   const Operator* OsrNormalEntry();
   const Operator* OsrLoopEntry();
   const Operator* OsrValue(int index);
+  const Operator* OsrGuard(OsrGuardType type);
 
   const Operator* Int32Constant(int32_t);
   const Operator* Int64Constant(int64_t);

src/compiler/instruction-selector.cc

@@ -1733,7 +1733,7 @@ void InstructionSelector::VisitIfException(Node* node) {
 
 void InstructionSelector::VisitOsrValue(Node* node) {
   OperandGenerator g(this);
-  int index = OpParameter<int>(node);
+  int index = OsrValueIndexOf(node->op());
   Emit(kArchNop,
        g.DefineAsLocation(node, linkage()->GetOsrValueLocation(index)));
 }

src/compiler/js-frame-specialization.cc

@@ -28,7 +28,7 @@ Reduction JSFrameSpecialization::Reduce(Node* node) {
 Reduction JSFrameSpecialization::ReduceOsrValue(Node* node) {
   DCHECK_EQ(IrOpcode::kOsrValue, node->opcode());
   Handle<Object> value;
-  int const index = OpParameter<int>(node);
+  int index = OsrValueIndexOf(node->op());
   int const parameters_count = frame()->ComputeParametersCount() + 1;
   if (index == Linkage::kOsrContextSpillSlotIndex) {
     value = handle(frame()->context(), isolate());

src/compiler/node-properties.cc

@@ -369,7 +369,7 @@ MaybeHandle<Context> NodeProperties::GetSpecializationNativeContext(
         return handle(context->native_context());
       }
       case IrOpcode::kOsrValue: {
-        int const index = OpParameter<int>(node);
+        int const index = OsrValueIndexOf(node->op());
         if (index == Linkage::kOsrContextSpillSlotIndex) {
           return native_context;
         }

src/compiler/opcodes.h

@@ -58,6 +58,7 @@
   V(Call)                 \
   V(Parameter)            \
   V(OsrValue)             \
+  V(OsrGuard)             \
   V(LoopExit)             \
   V(LoopExitValue)        \
   V(LoopExitEffect)       \

src/compiler/osr.cc

@@ -47,13 +47,14 @@ OsrHelper::OsrHelper(CompilationInfo* info)
     if (TRACE_COND) PrintF(__VA_ARGS__); \
   } while (false)
 
+namespace {
 
 // Peel outer loops and rewire the graph so that control reduction can
 // produce a properly formed graph.
-static void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
-                                 Zone* tmp_zone, Node* dead,
-                                 LoopTree* loop_tree, LoopTree::Loop* osr_loop,
-                                 Node* osr_normal_entry, Node* osr_loop_entry) {
+void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
+                          Zone* tmp_zone, Node* dead, LoopTree* loop_tree,
+                          LoopTree::Loop* osr_loop, Node* osr_normal_entry,
+                          Node* osr_loop_entry) {
   const size_t original_count = graph->NodeCount();
   AllNodes all(tmp_zone, graph);
   NodeVector tmp_inputs(tmp_zone);
@@ -93,7 +94,8 @@ static void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
         continue;
       }
       if (orig->InputCount() == 0 || orig->opcode() == IrOpcode::kParameter ||
-          orig->opcode() == IrOpcode::kOsrValue) {
+          orig->opcode() == IrOpcode::kOsrValue ||
+          orig->opcode() == IrOpcode::kOsrGuard) {
         // No need to copy leaf nodes or parameters.
         mapping->at(orig->id()) = orig;
         continue;
@@ -255,6 +257,41 @@ static void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
   }
 }
 
+void SetTypeForOsrValue(Node* osr_value, Node* loop,
+                        CommonOperatorBuilder* common) {
+  Node* osr_guard = nullptr;
+  for (Node* use : osr_value->uses()) {
+    if (use->opcode() == IrOpcode::kOsrGuard) {
+      DCHECK_EQ(use->InputAt(0), osr_value);
+      osr_guard = use;
+      break;
+    }
+  }
+
+  OsrGuardType guard_type = OsrGuardType::kAny;
+  // Find the phi that uses the OsrGuard node and get the type from
+  // there. Skip the search if the OsrGuard does not have value use
+  // (i.e., if there is other use beyond the effect use).
+  if (osr_guard->UseCount() > 1) {
+    Type* type = nullptr;
+    for (Node* use : osr_guard->uses()) {
+      if (use->opcode() == IrOpcode::kPhi) {
+        if (NodeProperties::GetControlInput(use) != loop) continue;
+        CHECK_NULL(type);
+        type = NodeProperties::GetType(use);
+      }
+    }
+    CHECK_NOT_NULL(type);
+
+    if (type->Is(Type::SignedSmall())) {
+      guard_type = OsrGuardType::kSignedSmall;
+    }
+  }
+
+  NodeProperties::ChangeOp(osr_guard, common->OsrGuard(guard_type));
+}
+
+}  // namespace
 
 void OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common,
                             Zone* tmp_zone) {
@@ -283,6 +320,12 @@ void OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common,
 
   CHECK(osr_loop);  // Should have found the OSR loop.
 
+  for (Node* use : osr_loop_entry->uses()) {
+    if (use->opcode() == IrOpcode::kOsrValue) {
+      SetTypeForOsrValue(use, osr_loop, common);
+    }
+  }
+
   // Analyze the graph to determine how deeply nested the OSR loop is.
   LoopTree* loop_tree = LoopFinder::BuildLoopTree(graph, tmp_zone);
 

src/compiler/pipeline.cc

@@ -859,7 +859,20 @@ struct TyperPhase {
   }
 };
 
-#ifdef DEBUG
+struct OsrTyperPhase {
+  static const char* phase_name() { return "osr typer"; }
+
+  void Run(PipelineData* data, Zone* temp_zone) {
+    NodeVector roots(temp_zone);
+    data->jsgraph()->GetCachedNodes(&roots);
+    // Dummy induction variable optimizer: at the moment, we do not try
+    // to compute loop variable bounds on OSR.
+    LoopVariableOptimizer induction_vars(data->jsgraph()->graph(),
+                                         data->common(), temp_zone);
+    Typer typer(data->isolate(), data->graph());
+    typer.Run(roots, &induction_vars);
+  }
+};
 
 struct UntyperPhase {
   static const char* phase_name() { return "untyper"; }
@@ -876,6 +889,12 @@ struct UntyperPhase {
       }
     };
 
+    NodeVector roots(temp_zone);
+    data->jsgraph()->GetCachedNodes(&roots);
+    for (Node* node : roots) {
+      NodeProperties::RemoveType(node);
+    }
+
     JSGraphReducer graph_reducer(data->jsgraph(), temp_zone);
     RemoveTypeReducer remove_type_reducer;
     AddReducer(data, &graph_reducer, &remove_type_reducer);
@@ -883,12 +902,15 @@ struct UntyperPhase {
   }
 };
 
-#endif  // DEBUG
-
 struct OsrDeconstructionPhase {
   static const char* phase_name() { return "OSR deconstruction"; }
 
   void Run(PipelineData* data, Zone* temp_zone) {
+    GraphTrimmer trimmer(temp_zone, data->graph());
+    NodeVector roots(temp_zone);
+    data->jsgraph()->GetCachedNodes(&roots);
+    trimmer.TrimGraph(roots.begin(), roots.end());
+
     OsrHelper osr_helper(data->info());
     osr_helper.Deconstruct(data->jsgraph(), data->common(), temp_zone);
   }
@@ -1495,7 +1517,11 @@ bool PipelineImpl::CreateGraph() {
 
   // Perform OSR deconstruction.
   if (info()->is_osr()) {
+    Run<OsrTyperPhase>();
+
     Run<OsrDeconstructionPhase>();
+
+    Run<UntyperPhase>();
     RunPrintAndVerify("OSR deconstruction", true);
   }
 

src/compiler/simplified-lowering.cc

@@ -161,7 +161,8 @@ void ReplaceEffectControlUses(Node* node, Node* effect, Node* control) {
     } else if (NodeProperties::IsEffectEdge(edge)) {
       edge.UpdateTo(effect);
     } else {
-      DCHECK(NodeProperties::IsValueEdge(edge));
+      DCHECK(NodeProperties::IsValueEdge(edge) ||
+             NodeProperties::IsContextEdge(edge));
     }
   }
 }
@@ -1266,6 +1267,30 @@ class RepresentationSelector {
     return;
   }
 
+  void VisitOsrGuard(Node* node) {
+    VisitInputs(node);
+
+    // Insert a dynamic check for the OSR value type if necessary.
+    switch (OsrGuardTypeOf(node->op())) {
+      case OsrGuardType::kUninitialized:
+        // At this point, we should always have a type for the OsrValue.
+        UNREACHABLE();
+        break;
+      case OsrGuardType::kSignedSmall:
+        if (lower()) {
+          NodeProperties::ChangeOp(node,
+                                   simplified()->CheckedTaggedToTaggedSigned());
+        }
+        return SetOutput(node, MachineRepresentation::kTaggedSigned);
+      case OsrGuardType::kAny:  // Nothing to check.
+        if (lower()) {
+          DeferReplacement(node, node->InputAt(0));
+        }
+        return SetOutput(node, MachineRepresentation::kTagged);
+    }
+    UNREACHABLE();
+  }
+
   // Dispatching routine for visiting the node {node} with the usage {use}.
   // Depending on the operator, propagate new usage info to the inputs.
   void VisitNode(Node* node, Truncation truncation,
@@ -2409,6 +2434,9 @@ class RepresentationSelector {
         return;
       }
 
+      case IrOpcode::kOsrGuard:
+        return VisitOsrGuard(node);
+
       // Operators with all inputs tagged and no or tagged output have uniform
       // handling.
       case IrOpcode::kEnd:
@@ -2427,9 +2455,9 @@ class RepresentationSelector {
       case IrOpcode::kThrow:
       case IrOpcode::kBeginRegion:
       case IrOpcode::kFinishRegion:
-      case IrOpcode::kOsrValue:
       case IrOpcode::kProjection:
       case IrOpcode::kObjectState:
+      case IrOpcode::kOsrValue:
 // All JavaScript operators except JSToNumber have uniform handling.
 #define OPCODE_CASE(name) case IrOpcode::k##name:
         JS_SIMPLE_BINOP_LIST(OPCODE_CASE)

src/compiler/typer.cc

@@ -550,6 +550,19 @@ Type* Typer::Visitor::TypeParameter(Node* node) { return Type::Any(); }
 
 Type* Typer::Visitor::TypeOsrValue(Node* node) { return Type::Any(); }
 
+Type* Typer::Visitor::TypeOsrGuard(Node* node) {
+  switch (OsrGuardTypeOf(node->op())) {
+    case OsrGuardType::kUninitialized:
+      return Type::None();
+    case OsrGuardType::kSignedSmall:
+      return Type::SignedSmall();
+    case OsrGuardType::kAny:
+      return Type::Any();
+  }
+  UNREACHABLE();
+  return nullptr;
+}
+
 Type* Typer::Visitor::TypeRetain(Node* node) {
   UNREACHABLE();
   return nullptr;

src/compiler/verifier.cc

@@ -375,6 +375,23 @@ void Verifier::Visitor::Check(Node* node) {
       // Type is merged from other values in the graph and could be any.
       CheckTypeIs(node, Type::Any());
       break;
+    case IrOpcode::kOsrGuard:
+      // OSR values have a value and a control input.
+      CHECK_EQ(1, value_count);
+      CHECK_EQ(1, effect_count);
+      CHECK_EQ(1, control_count);
+      switch (OsrGuardTypeOf(node->op())) {
+        case OsrGuardType::kUninitialized:
+          CheckTypeIs(node, Type::None());
+          break;
+        case OsrGuardType::kSignedSmall:
+          CheckTypeIs(node, Type::SignedSmall());
+          break;
+        case OsrGuardType::kAny:
+          CheckTypeIs(node, Type::Any());
+          break;
+      }
+      break;
     case IrOpcode::kProjection: {
       // Projection has an input that produces enough values.
       int index = static_cast<int>(ProjectionIndexOf(node->op()));

test/cctest/BUILD.gn

@@ -40,7 +40,6 @@ v8_executable("cctest") {
     "compiler/test-multiple-return.cc",
     "compiler/test-node.cc",
     "compiler/test-operator.cc",
-    "compiler/test-osr.cc",
     "compiler/test-representation-change.cc",
     "compiler/test-run-bytecode-graph-builder.cc",
     "compiler/test-run-calls-to-external-references.cc",

test/cctest/cctest.gyp

@@ -60,7 +60,6 @@
       'compiler/test-multiple-return.cc',
       'compiler/test-node.cc',
       'compiler/test-operator.cc',
-      'compiler/test-osr.cc',
       'compiler/test-representation-change.cc',
       'compiler/test-run-bytecode-graph-builder.cc',
       'compiler/test-run-calls-to-external-references.cc',

test/mjsunit/compiler/osr-typing-debug-change.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 --expose-debug-as debug
+
+var Debug = debug.Debug;
+
+var changed = false;
+
+function listenerSetJToResult(
+    event, exec_state, event_data, data) {
+
+  if (event == Debug.DebugEvent.Break) {
+    var scope = exec_state.frame(1).scope(0);
+    var newval = "result";
+    try {
+      scope.setVariableValue("j", newval);
+      changed = true;
+    } catch(e) {
+      changed = false;
+    }
+  }
+}
+
+Debug.setListener(listenerSetJToResult);
+
+function g() { debugger; }
+%NeverOptimizeFunction(g);
+
+function ChangeSmiConstantAndOsr() {
+  var j = 1;
+  for (var i = 0; i < 4; i++) {
+    if (i == 2) {
+      %OptimizeOsr();
+      g();
+    }
+  }
+  return j;
+}
+var r1 = ChangeSmiConstantAndOsr();
+if (changed) {
+  assertEquals("result", r1);
+} else {
+  assertEquals(1, r1);
+}
+
+function ChangeFloatConstantAndOsr() {
+  var j = 0.1;
+  for (var i = 0; i < 4; i++) {
+    if (i == 2) {
+      %OptimizeOsr();
+      g();
+    }
+  }
+  return j;
+}
+var r2 = ChangeFloatConstantAndOsr();
+if (changed) {
+  assertEquals("result", r2);
+} else {
+  assertEquals(0.1, r2);
+}
+
+function ChangeFloatVarAndOsr() {
+  var j = 0.1;
+  for (var i = 0; i < 4; i++) {
+    j = j + 0.1;
+    if (i == 2) {
+      %OptimizeOsr();
+      g();
+    }
+  }
+  return j;
+}
+var r3 = ChangeFloatVarAndOsr();
+if (changed) {
+  assertEquals("result0.1", r3);
+} else {
+  assertEquals(0.5, r3);
+}
+
+var counter = 0;
+var o = { toString : function() { counter++; return 100; } };
+
+function listenerSetJToObject(
+    event, exec_state, event_data, data) {
+  if (event == Debug.DebugEvent.Break) {
+    var scope = exec_state.frame(1).scope(0);
+    try {
+      scope.setVariableValue("j", o);
+      changed = true;
+    } catch(e) {
+      changed = false;
+    }
+  }
+}
+
+Debug.setListener(listenerSetJToObject);
+
+function ChangeIntVarAndOsr() {
+  var j = 1;
+  for (var i = 0; i < 4; i++) {
+    j = j + 1|0;
+    if (i == 2) {
+      %OptimizeOsr();
+      g();
+    }
+  }
+  return j;
+}
+
+var r4 = ChangeIntVarAndOsr();
+if (changed) {
+  assertEquals(101, r4);
+  assertEquals(1, counter);
+} else {
+  assertEquals(5, r4);
+}