Revert "[turbofan] Add new JSFrameSpecialization reducer."
authormachenbach <machenbach@chromium.org>
Mon, 6 Jul 2015 10:01:27 +0000 (03:01 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 6 Jul 2015 10:01:42 +0000 (10:01 +0000)
Also revert "[turbofan] Perform OSR deconstruction early and remove type propagation."

This reverts commit b0a852e8c2ce2add1ba8a26d572aff39af0968a3.

This reverts commit cdbb6c485b8d07fd4ad1cb000d54a937507e3b3e.

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
src/compiler.cc
src/compiler.h
src/compiler/js-frame-specialization.cc [deleted file]
src/compiler/js-frame-specialization.h [deleted file]
src/compiler/osr.cc
src/compiler/pipeline.cc
src/compiler/typer.cc
src/runtime/runtime-compiler.cc
test/cctest/compiler/test-osr.cc
tools/gyp/v8.gyp

index a18161b..882e2bc 100644 (file)
--- a/BUILD.gn
+++ b/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",
index 0f3ebe0..495ba4f 100644 (file)
@@ -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;
-  // Frame specialization implies context specialization.
-  DCHECK(!info->is_frame_specializing());
+  if (code->is_turbofanned() && info->is_context_specializing()) return;
 
   // 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,
-                                             JavaScriptFrame* osr_frame) {
+                                             BailoutId osr_ast_id) {
   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();
   }
 
index 45863f6..e1e4be3 100644 (file)
@@ -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 << 11,
-    kTypingEnabled = 1 << 12,
-    kDisableFutureOptimization = 1 << 13,
-    kSplittingEnabled = 1 << 14,
-    kTypeFeedbackEnabled = 1 << 15,
-    kDeoptimizationEnabled = 1 << 16,
-    kSourcePositionsEnabled = 1 << 17,
-    kFirstCompile = 1 << 18,
+    kInliningEnabled = 1 << 10,
+    kTypingEnabled = 1 << 11,
+    kDisableFutureOptimization = 1 << 12,
+    kSplittingEnabled = 1 << 13,
+    kTypeFeedbackEnabled = 1 << 14,
+    kDeoptimizationEnabled = 1 << 15,
+    kSourcePositionsEnabled = 1 << 16,
+    kFirstCompile = 1 << 17,
   };
 
   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,
-      ConcurrencyMode mode, BailoutId osr_ast_id = BailoutId::None(),
-      JavaScriptFrame* osr_frame = nullptr);
+      Handle<JSFunction> function,
+      Handle<Code> current_code,
+      ConcurrencyMode mode,
+      BailoutId osr_ast_id = BailoutId::None());
 
   // Generate and return code from previously queued optimization job.
   // On failure, return the empty handle.
diff --git a/src/compiler/js-frame-specialization.cc b/src/compiler/js-frame-specialization.cc
deleted file mode 100644 (file)
index 98b1827..0000000
+++ /dev/null
@@ -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
diff --git a/src/compiler/js-frame-specialization.h b/src/compiler/js-frame-specialization.h
deleted file mode 100644 (file)
index c6fc561..0000000
+++ /dev/null
@@ -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_
index 6c4c49d..fb5716a 100644 (file)
@@ -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);
 
index 66f3bf0..7ced8e6 100644 (file)
@@ -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.
index 3f4bd27..fa0726c 100644 (file)
@@ -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());
 }
 
index 0b6677a..ba92faa 100644 (file)
@@ -219,12 +219,11 @@ 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 = (isolate->concurrent_osr_enabled() &&
-                                    !function->shared()->asm_function() &&
-                                    function->shared()->ast_node_count() > 512)
-                                       ? Compiler::CONCURRENT
-                                       : Compiler::NOT_CONCURRENT;
+  Compiler::ConcurrencyMode mode =
+      isolate->concurrent_osr_enabled() &&
+              (function->shared()->ast_node_count() > 512)
+          ? Compiler::CONCURRENT
+          : Compiler::NOT_CONCURRENT;
   Handle<Code> result = Handle<Code>::null();
 
   OptimizedCompileJob* job = 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(
-        function, caller_code, mode, ast_id,
-        (mode == Compiler::NOT_CONCURRENT) ? frame : nullptr);
+    MaybeHandle<Code> maybe_result =
+        Compiler::GetOptimizedCode(function, caller_code, mode, ast_id);
     if (maybe_result.ToHandle(&result) &&
         result.is_identical_to(isolate->builtins()->InOptimizationQueue())) {
       // Optimization is queued.  Return to check later.
index 80dbccc..77ade91 100644 (file)
@@ -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);
index 576c871..b1a30fd 100644 (file)
         '../../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',