Revert "[turbofan] Add new JSFrameSpecialization reducer."

Also revert "[turbofan] Perform OSR deconstruction early and remove type propagation."

This reverts commit b0a852e8c2.

This reverts commit cdbb6c485b.

NOTRY=true
NOTREECHECKS=true
BUG=v8:4273
LOG=n
TBR=bmeurer@chromium.org

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

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

BUILD.gn

@@ -672,8 +672,6 @@ source_set("v8_base") {
     "src/compiler/js-builtin-reducer.h",
     "src/compiler/js-context-specialization.cc",
     "src/compiler/js-context-specialization.h",
-    "src/compiler/js-frame-specialization.cc",
-    "src/compiler/js-frame-specialization.h",
     "src/compiler/js-generic-lowering.cc",
     "src/compiler/js-generic-lowering.h",
     "src/compiler/js-graph.cc",

src/compiler.cc

@@ -394,7 +394,6 @@ OptimizedCompileJob::Status OptimizedCompileJob::CreateGraph() {
     }
 
     if (info()->shared_info()->asm_function()) {
-      if (info()->osr_frame()) info()->MarkAsFrameSpecializing();
       info()->MarkAsContextSpecializing();
     } else if (FLAG_turbo_type_feedback) {
       info()->MarkAsTypeFeedbackEnabled();
@@ -713,9 +712,7 @@ static void InsertCodeIntoOptimizedCodeMap(CompilationInfo* info) {
   if (code->kind() != Code::OPTIMIZED_FUNCTION) return;  // Nothing to do.
 
   // Context specialization folds-in the context, so no sharing can occur.
-  if (info->is_context_specializing()) return;
+  if (code->is_turbofanned() && info->is_context_specializing()) return;
-  // Frame specialization implies context specialization.
-  DCHECK(!info->is_frame_specializing());
 
   // Do not cache bound functions.
   Handle<JSFunction> function = info->closure();
@@ -1472,8 +1469,7 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
 MaybeHandle<Code> Compiler::GetOptimizedCode(Handle<JSFunction> function,
                                              Handle<Code> current_code,
                                              ConcurrencyMode mode,
-                                             BailoutId osr_ast_id,
+                                             BailoutId osr_ast_id) {
-                                             JavaScriptFrame* osr_frame) {
   Handle<Code> cached_code;
   if (GetCodeFromOptimizedCodeMap(
           function, osr_ast_id).ToHandle(&cached_code)) {
@@ -1531,7 +1527,6 @@ MaybeHandle<Code> Compiler::GetOptimizedCode(Handle<JSFunction> function,
       return isolate->builtins()->InOptimizationQueue();
     }
   } else {
-    info->set_osr_frame(osr_frame);
     if (GetOptimizedCodeNow(info.get())) return info->code();
   }
 

src/compiler.h

@@ -17,7 +17,6 @@ namespace internal {
 
 class AstValueFactory;
 class HydrogenCodeStub;
-class JavaScriptFrame;
 class ParseInfo;
 class ScriptData;
 
@@ -123,15 +122,14 @@ class CompilationInfo {
     kCompilingForDebugging = 1 << 7,
     kSerializing = 1 << 8,
     kContextSpecializing = 1 << 9,
-    kFrameSpecializing = 1 << 10,
+    kInliningEnabled = 1 << 10,
-    kInliningEnabled = 1 << 11,
+    kTypingEnabled = 1 << 11,
-    kTypingEnabled = 1 << 12,
+    kDisableFutureOptimization = 1 << 12,
-    kDisableFutureOptimization = 1 << 13,
+    kSplittingEnabled = 1 << 13,
-    kSplittingEnabled = 1 << 14,
+    kTypeFeedbackEnabled = 1 << 14,
-    kTypeFeedbackEnabled = 1 << 15,
+    kDeoptimizationEnabled = 1 << 15,
-    kDeoptimizationEnabled = 1 << 16,
+    kSourcePositionsEnabled = 1 << 16,
-    kSourcePositionsEnabled = 1 << 17,
+    kFirstCompile = 1 << 17,
-    kFirstCompile = 1 << 18,
   };
 
   explicit CompilationInfo(ParseInfo* parse_info);
@@ -219,10 +217,6 @@ class CompilationInfo {
 
   bool is_context_specializing() const { return GetFlag(kContextSpecializing); }
 
-  void MarkAsFrameSpecializing() { SetFlag(kFrameSpecializing); }
-
-  bool is_frame_specializing() const { return GetFlag(kFrameSpecializing); }
-
   void MarkAsTypeFeedbackEnabled() { SetFlag(kTypeFeedbackEnabled); }
 
   bool is_type_feedback_enabled() const {
@@ -394,8 +388,6 @@ class CompilationInfo {
     DCHECK(height >= 0);
     osr_expr_stack_height_ = height;
   }
-  JavaScriptFrame* osr_frame() const { return osr_frame_; }
-  void set_osr_frame(JavaScriptFrame* osr_frame) { osr_frame_ = osr_frame; }
 
 #if DEBUG
   void PrintAstForTesting();
@@ -500,9 +492,6 @@ class CompilationInfo {
 
   int osr_expr_stack_height_;
 
-  // The current OSR frame for specialization or {nullptr}.
-  JavaScriptFrame* osr_frame_ = nullptr;
-
   Type::FunctionType* function_type_;
 
   DISALLOW_COPY_AND_ASSIGN(CompilationInfo);
@@ -673,9 +662,10 @@ class Compiler : public AllStatic {
   // In the latter case, return the InOptimizationQueue builtin.  On failure,
   // return the empty handle.
   MUST_USE_RESULT static MaybeHandle<Code> GetOptimizedCode(
-      Handle<JSFunction> function, Handle<Code> current_code,
+      Handle<JSFunction> function,
-      ConcurrencyMode mode, BailoutId osr_ast_id = BailoutId::None(),
+      Handle<Code> current_code,
-      JavaScriptFrame* osr_frame = nullptr);
+      ConcurrencyMode mode,
+      BailoutId osr_ast_id = BailoutId::None());
 
   // Generate and return code from previously queued optimization job.
   // On failure, return the empty handle.

src/compiler/js-frame-specialization.cc

@@ -1,69 +0,0 @@
-// 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.
-
-#include "src/compiler/js-frame-specialization.h"
-
-#include "src/compiler/js-graph.h"
-#include "src/compiler/linkage.h"
-#include "src/frames-inl.h"
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-Reduction JSFrameSpecialization::Reduce(Node* node) {
-  switch (node->opcode()) {
-    case IrOpcode::kOsrValue:
-      return ReduceOsrValue(node);
-    case IrOpcode::kParameter:
-      return ReduceParameter(node);
-    default:
-      break;
-  }
-  return NoChange();
-}
-
-
-Reduction JSFrameSpecialization::ReduceOsrValue(Node* node) {
-  DCHECK_EQ(IrOpcode::kOsrValue, node->opcode());
-  DisallowHeapAllocation no_gc;
-  Object* object;
-  int const index = OpParameter<int>(node);
-  int const parameters_count = frame()->ComputeParametersCount() + 1;
-  if (index == Linkage::kOsrContextSpillSlotIndex) {
-    object = frame()->context();
-  } else if (index >= parameters_count) {
-    object = frame()->GetExpression(index - parameters_count);
-  } else {
-    // The OsrValue index 0 is the receiver.
-    object = index ? frame()->GetParameter(index - 1) : frame()->receiver();
-  }
-  return Replace(jsgraph()->Constant(handle(object, isolate())));
-}
-
-
-Reduction JSFrameSpecialization::ReduceParameter(Node* node) {
-  DCHECK_EQ(IrOpcode::kParameter, node->opcode());
-  DisallowHeapAllocation no_gc;
-  Object* object;
-  int const index = ParameterIndexOf(node->op());
-  int const parameters_count = frame()->ComputeParametersCount() + 1;
-  if (index == Linkage::kJSFunctionCallClosureParamIndex) {
-    object = frame()->function();
-  } else if (index == parameters_count) {
-    // The Parameter index (arity + 1) is the context.
-    object = frame()->context();
-  } else {
-    // The Parameter index 0 is the receiver.
-    object = index ? frame()->GetParameter(index - 1) : frame()->receiver();
-  }
-  return Replace(jsgraph()->Constant(handle(object, isolate())));
-}
-
-
-Isolate* JSFrameSpecialization::isolate() const { return jsgraph()->isolate(); }
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8

src/compiler/js-frame-specialization.h

@@ -1,44 +0,0 @@
-// 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.
-
-#ifndef V8_COMPILER_JS_FRAME_SPECIALIZATION_H_
-#define V8_COMPILER_JS_FRAME_SPECIALIZATION_H_
-
-#include "src/compiler/graph-reducer.h"
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-// Forward declarations.
-class JSGraph;
-
-
-class JSFrameSpecialization final : public Reducer {
- public:
-  JSFrameSpecialization(JavaScriptFrame const* frame, JSGraph* jsgraph)
-      : frame_(frame), jsgraph_(jsgraph) {}
-  ~JSFrameSpecialization() final {}
-
-  Reduction Reduce(Node* node) final;
-
- private:
-  Reduction ReduceOsrValue(Node* node);
-  Reduction ReduceParameter(Node* node);
-
-  Isolate* isolate() const;
-  JavaScriptFrame const* frame() const { return frame_; }
-  JSGraph* jsgraph() const { return jsgraph_; }
-
-  JavaScriptFrame const* const frame_;
-  JSGraph* const jsgraph_;
-
-  DISALLOW_COPY_AND_ASSIGN(JSFrameSpecialization);
-};
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_COMPILER_JS_FRAME_SPECIALIZATION_H_

src/compiler/osr.cc

@@ -249,6 +249,40 @@ static void PeelOuterLoopsForOsr(Graph* graph, CommonOperatorBuilder* common,
 }
 
 
+static void TransferOsrValueTypesFromLoopPhis(Zone* zone, Node* osr_loop_entry,
+                                              Node* osr_loop) {
+  // Find the index of the osr loop entry into the loop.
+  int index = 0;
+  for (index = 0; index < osr_loop->InputCount(); index++) {
+    if (osr_loop->InputAt(index) == osr_loop_entry) break;
+  }
+  if (index == osr_loop->InputCount()) return;
+
+  for (Node* osr_value : osr_loop_entry->uses()) {
+    if (osr_value->opcode() != IrOpcode::kOsrValue) continue;
+    bool unknown = true;
+    for (Node* phi : osr_value->uses()) {
+      if (phi->opcode() != IrOpcode::kPhi) continue;
+      if (NodeProperties::GetControlInput(phi) != osr_loop) continue;
+      if (phi->InputAt(index) != osr_value) continue;
+      if (NodeProperties::IsTyped(phi)) {
+        // Transfer the type from the phi to the OSR value itself.
+        Bounds phi_bounds = NodeProperties::GetBounds(phi);
+        if (unknown) {
+          NodeProperties::SetBounds(osr_value, phi_bounds);
+        } else {
+          Bounds osr_bounds = NodeProperties::GetBounds(osr_value);
+          NodeProperties::SetBounds(osr_value,
+                                    Bounds::Both(phi_bounds, osr_bounds, zone));
+        }
+        unknown = false;
+      }
+    }
+    if (unknown) NodeProperties::SetBounds(osr_value, Bounds::Unbounded(zone));
+  }
+}
+
+
 void OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common,
                             Zone* tmp_zone) {
   Graph* graph = jsgraph->graph();
@@ -279,6 +313,9 @@ void OsrHelper::Deconstruct(JSGraph* jsgraph, CommonOperatorBuilder* common,
 
   CHECK(osr_loop);  // Should have found the OSR loop.
 
+  // Transfer the types from loop phis to the OSR values which flow into them.
+  TransferOsrValueTypesFromLoopPhis(graph->zone(), osr_loop_entry, osr_loop);
+
   // Analyze the graph to determine how deeply nested the OSR loop is.
   LoopTree* loop_tree = LoopFinder::BuildLoopTree(graph, tmp_zone);
 

src/compiler/pipeline.cc

@@ -26,7 +26,6 @@
 #include "src/compiler/instruction-selector.h"
 #include "src/compiler/js-builtin-reducer.h"
 #include "src/compiler/js-context-specialization.h"
-#include "src/compiler/js-frame-specialization.h"
 #include "src/compiler/js-generic-lowering.h"
 #include "src/compiler/js-inlining.h"
 #include "src/compiler/js-intrinsic-lowering.h"
@@ -497,17 +496,12 @@ struct InliningPhase {
     CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
                                          data->common(), data->machine());
     JSContextSpecializer context_specializer(&graph_reducer, data->jsgraph());
-    JSFrameSpecialization frame_specialization(data->info()->osr_frame(),
-                                               data->jsgraph());
     JSInliner inliner(&graph_reducer, data->info()->is_inlining_enabled()
                                           ? JSInliner::kGeneralInlining
                                           : JSInliner::kRestrictedInlining,
                       temp_zone, data->info(), data->jsgraph());
     AddReducer(data, &graph_reducer, &dead_code_elimination);
     AddReducer(data, &graph_reducer, &common_reducer);
-    if (data->info()->is_frame_specializing()) {
-      AddReducer(data, &graph_reducer, &frame_specialization);
-    }
     if (data->info()->is_context_specializing()) {
       AddReducer(data, &graph_reducer, &context_specializer);
     }
@@ -1041,12 +1035,6 @@ Handle<Code> Pipeline::GenerateCode() {
   if (data.compilation_failed()) return Handle<Code>::null();
   RunPrintAndVerify("Initial untyped", true);
 
-  // Perform OSR deconstruction.
-  if (info()->is_osr()) {
-    Run<OsrDeconstructionPhase>();
-    RunPrintAndVerify("OSR deconstruction", true);
-  }
-
   // Perform context specialization and inlining (if enabled).
   Run<InliningPhase>();
   RunPrintAndVerify("Inlined", true);
@@ -1083,6 +1071,11 @@ Handle<Code> Pipeline::GenerateCode() {
       RunPrintAndVerify("Loop peeled");
     }
 
+    if (info()->is_osr()) {
+      Run<OsrDeconstructionPhase>();
+      RunPrintAndVerify("OSR deconstruction");
+    }
+
     if (info()->is_type_feedback_enabled()) {
       Run<JSTypeFeedbackPhase>();
       RunPrintAndVerify("JSType feedback");
@@ -1102,6 +1095,12 @@ Handle<Code> Pipeline::GenerateCode() {
     Run<ChangeLoweringPhase>();
     // TODO(jarin, rossberg): Remove UNTYPED once machine typing works.
     RunPrintAndVerify("Lowered changes", true);
+  } else {
+    if (info()->is_osr()) {
+      Run<OsrDeconstructionPhase>();
+      if (info()->bailout_reason() != kNoReason) return Handle<Code>::null();
+      RunPrintAndVerify("OSR deconstruction", true);
+    }
   }
 
   // Lower any remaining generic JSOperators.

src/compiler/typer.cc

@@ -582,6 +582,16 @@ Bounds Typer::Visitor::TypeParameter(Node* node) {
 
 
 Bounds Typer::Visitor::TypeOsrValue(Node* node) {
+  if (node->InputAt(0)->opcode() == IrOpcode::kOsrLoopEntry) {
+    // Before deconstruction, OSR values have type {None} to avoid polluting
+    // the types of phis and other nodes in the graph.
+    return Bounds(Type::None(), Type::None());
+  }
+  if (NodeProperties::IsTyped(node)) {
+    // After deconstruction, OSR values may have had a type explicitly set.
+    return NodeProperties::GetBounds(node);
+  }
+  // Otherwise, be conservative.
   return Bounds::Unbounded(zone());
 }
 

src/runtime/runtime-compiler.cc

@@ -219,10 +219,9 @@ RUNTIME_FUNCTION(Runtime_CompileForOnStackReplacement) {
   BailoutId ast_id = caller_code->TranslatePcOffsetToAstId(pc_offset);
   DCHECK(!ast_id.IsNone());
 
-  // Disable concurrent OSR for asm.js, to enable frame specialization.
+  Compiler::ConcurrencyMode mode =
-  Compiler::ConcurrencyMode mode = (isolate->concurrent_osr_enabled() &&
+      isolate->concurrent_osr_enabled() &&
-                                    !function->shared()->asm_function() &&
+              (function->shared()->ast_node_count() > 512)
-                                    function->shared()->ast_node_count() > 512)
           ? Compiler::CONCURRENT
           : Compiler::NOT_CONCURRENT;
   Handle<Code> result = Handle<Code>::null();
@@ -259,9 +258,8 @@ RUNTIME_FUNCTION(Runtime_CompileForOnStackReplacement) {
       function->PrintName();
       PrintF(" at AST id %d]\n", ast_id.ToInt());
     }
-    MaybeHandle<Code> maybe_result = Compiler::GetOptimizedCode(
+    MaybeHandle<Code> maybe_result =
-        function, caller_code, mode, ast_id,
+        Compiler::GetOptimizedCode(function, caller_code, mode, ast_id);
-        (mode == Compiler::NOT_CONCURRENT) ? frame : nullptr);
     if (maybe_result.ToHandle(&result) &&
         result.is_identical_to(isolate->builtins()->InOptimizationQueue())) {
       // Optimization is queued.  Return to check later.

test/cctest/compiler/test-osr.cc

@@ -165,6 +165,30 @@ TEST(Deconstruct_osr1) {
 }
 
 
+TEST(Deconstruct_osr1_type) {
+  OsrDeconstructorTester T(1);
+
+  Node* loop = T.NewOsrLoop(1);
+  Node* osr_phi =
+      T.NewOsrPhi(loop, T.jsgraph.OneConstant(), 0, T.jsgraph.ZeroConstant());
+  Type* type = Type::Signed32();
+  NodeProperties::SetBounds(osr_phi, Bounds(type, type));
+
+  Node* ret = T.graph.NewNode(T.common.Return(), osr_phi, T.start, loop);
+  T.graph.SetEnd(ret);
+
+  OsrHelper helper(0, 0);
+  helper.Deconstruct(&T.jsgraph, &T.common, T.main_zone());
+
+  CHECK_EQ(type, NodeProperties::GetBounds(T.osr_values[0]).lower);
+  CHECK_EQ(type, NodeProperties::GetBounds(T.osr_values[0]).upper);
+
+  CheckInputs(loop, T.start, loop);
+  CheckInputs(osr_phi, T.osr_values[0], T.jsgraph.ZeroConstant(), loop);
+  CheckInputs(ret, osr_phi, T.start, loop);
+}
+
+
 TEST(Deconstruct_osr_remove_prologue) {
   OsrDeconstructorTester T(1);
   Diamond d(&T.graph, &T.common, T.p0);

tools/gyp/v8.gyp

@@ -505,8 +505,6 @@
         '../../src/compiler/js-builtin-reducer.h',
         '../../src/compiler/js-context-specialization.cc',
         '../../src/compiler/js-context-specialization.h',
-        '../../src/compiler/js-frame-specialization.cc',
-        '../../src/compiler/js-frame-specialization.h',
         '../../src/compiler/js-generic-lowering.cc',
         '../../src/compiler/js-generic-lowering.h',
         '../../src/compiler/js-graph.cc',