From: machenbach Date: Mon, 6 Jul 2015 10:01:27 +0000 (-0700) Subject: Revert "[turbofan] Add new JSFrameSpecialization reducer." X-Git-Tag: upstream/4.7.83~1560 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9e71cdba48c484c7423fccb969d896307a93e289;p=platform%2Fupstream%2Fv8.git Revert "[turbofan] Add new JSFrameSpecialization reducer." 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} --- diff --git a/BUILD.gn b/BUILD.gn index a18161b..882e2bc 100644 --- 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", diff --git a/src/compiler.cc b/src/compiler.cc index 0f3ebe0..495ba4f 100644 --- a/src/compiler.cc +++ b/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; - // 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 function = info->closure(); @@ -1472,8 +1469,7 @@ Handle Compiler::GetSharedFunctionInfo( MaybeHandle Compiler::GetOptimizedCode(Handle function, Handle current_code, ConcurrencyMode mode, - BailoutId osr_ast_id, - JavaScriptFrame* osr_frame) { + BailoutId osr_ast_id) { Handle cached_code; if (GetCodeFromOptimizedCodeMap( function, osr_ast_id).ToHandle(&cached_code)) { @@ -1531,7 +1527,6 @@ MaybeHandle Compiler::GetOptimizedCode(Handle function, return isolate->builtins()->InOptimizationQueue(); } } else { - info->set_osr_frame(osr_frame); if (GetOptimizedCodeNow(info.get())) return info->code(); } diff --git a/src/compiler.h b/src/compiler.h index 45863f6..e1e4be3 100644 --- a/src/compiler.h +++ b/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 << 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 GetOptimizedCode( - Handle function, Handle current_code, - ConcurrencyMode mode, BailoutId osr_ast_id = BailoutId::None(), - JavaScriptFrame* osr_frame = nullptr); + Handle function, + Handle 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 index 98b1827..0000000 --- a/src/compiler/js-frame-specialization.cc +++ /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(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 index c6fc561..0000000 --- a/src/compiler/js-frame-specialization.h +++ /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_ diff --git a/src/compiler/osr.cc b/src/compiler/osr.cc index 6c4c49d..fb5716a 100644 --- a/src/compiler/osr.cc +++ b/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); diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index 66f3bf0..7ced8e6 100644 --- a/src/compiler/pipeline.cc +++ b/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 Pipeline::GenerateCode() { if (data.compilation_failed()) return Handle::null(); RunPrintAndVerify("Initial untyped", true); - // Perform OSR deconstruction. - if (info()->is_osr()) { - Run(); - RunPrintAndVerify("OSR deconstruction", true); - } - // Perform context specialization and inlining (if enabled). Run(); RunPrintAndVerify("Inlined", true); @@ -1083,6 +1071,11 @@ Handle Pipeline::GenerateCode() { RunPrintAndVerify("Loop peeled"); } + if (info()->is_osr()) { + Run(); + RunPrintAndVerify("OSR deconstruction"); + } + if (info()->is_type_feedback_enabled()) { Run(); RunPrintAndVerify("JSType feedback"); @@ -1102,6 +1095,12 @@ Handle Pipeline::GenerateCode() { Run(); // TODO(jarin, rossberg): Remove UNTYPED once machine typing works. RunPrintAndVerify("Lowered changes", true); + } else { + if (info()->is_osr()) { + Run(); + if (info()->bailout_reason() != kNoReason) return Handle::null(); + RunPrintAndVerify("OSR deconstruction", true); + } } // Lower any remaining generic JSOperators. diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index 3f4bd27..fa0726c 100644 --- a/src/compiler/typer.cc +++ b/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()); } diff --git a/src/runtime/runtime-compiler.cc b/src/runtime/runtime-compiler.cc index 0b6677a..ba92faa 100644 --- a/src/runtime/runtime-compiler.cc +++ b/src/runtime/runtime-compiler.cc @@ -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 result = Handle::null(); OptimizedCompileJob* job = NULL; @@ -259,9 +258,8 @@ RUNTIME_FUNCTION(Runtime_CompileForOnStackReplacement) { function->PrintName(); PrintF(" at AST id %d]\n", ast_id.ToInt()); } - MaybeHandle maybe_result = Compiler::GetOptimizedCode( - function, caller_code, mode, ast_id, - (mode == Compiler::NOT_CONCURRENT) ? frame : nullptr); + MaybeHandle 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. diff --git a/test/cctest/compiler/test-osr.cc b/test/cctest/compiler/test-osr.cc index 80dbccc..77ade91 100644 --- a/test/cctest/compiler/test-osr.cc +++ b/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); diff --git a/tools/gyp/v8.gyp b/tools/gyp/v8.gyp index 576c871..b1a30fd 100644 --- a/tools/gyp/v8.gyp +++ b/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',