From a4f060278f18c38bb77d5cb41c72d7f6d3f270f6 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Wed, 17 Jun 2015 05:25:02 -0700 Subject: [PATCH] [turbofan] Fix life time and use of the Typer. Currently the Typer is installed on the Graph, no matter if we actually use the types or not (read: even in the generic pipeline). Also the Typer tries hard to eagerly type nodes during graph building, which takes time, just to remove those types later again, and retype everything from scratch. Plus this is inconsistent, since it only applies to the outermost graph, not the inlined graphs (which are eagerly typed once the nodes are copied). So in summary, what's currently implemented is neither useful nor well defined, so for now we stick to the full typing approach until a proper design for eager typing is available that will actually benefit us. R=rossberg@chromium.org,mstarzinger@chromium.org,jarin@chromium.org Review URL: https://codereview.chromium.org/1192553002 Cr-Commit-Position: refs/heads/master@{#29086} --- src/compiler/pipeline.cc | 25 +++++++++++++------------ src/compiler/typer.cc | 25 ++++--------------------- src/compiler/typer.h | 2 ++ 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index 7b74592..84995be 100644 --- a/src/compiler/pipeline.cc +++ b/src/compiler/pipeline.cc @@ -79,7 +79,6 @@ class PipelineData { javascript_(nullptr), jsgraph_(nullptr), js_type_feedback_(nullptr), - typer_(nullptr), schedule_(nullptr), instruction_zone_scope_(zone_pool_), instruction_zone_(instruction_zone_scope_.zone()), @@ -98,7 +97,6 @@ class PipelineData { javascript_ = new (graph_zone_) JSOperatorBuilder(graph_zone_); jsgraph_ = new (graph_zone_) JSGraph(isolate_, graph_, common_, javascript_, machine_); - typer_.Reset(new Typer(isolate_, graph_, info_->context())); } // For machine graph testing entry point. @@ -121,7 +119,6 @@ class PipelineData { javascript_(nullptr), jsgraph_(nullptr), js_type_feedback_(nullptr), - typer_(nullptr), schedule_(schedule), instruction_zone_scope_(zone_pool_), instruction_zone_(instruction_zone_scope_.zone()), @@ -150,7 +147,6 @@ class PipelineData { javascript_(nullptr), jsgraph_(nullptr), js_type_feedback_(nullptr), - typer_(nullptr), schedule_(nullptr), instruction_zone_scope_(zone_pool_), instruction_zone_(sequence->zone()), @@ -194,7 +190,6 @@ class PipelineData { void set_js_type_feedback(JSTypeFeedbackTable* js_type_feedback) { js_type_feedback_ = js_type_feedback; } - Typer* typer() const { return typer_.get(); } LoopAssignmentAnalysis* loop_assignment() const { return loop_assignment_; } void set_loop_assignment(LoopAssignmentAnalysis* loop_assignment) { @@ -220,7 +215,6 @@ class PipelineData { void DeleteGraphZone() { // Destroy objects with destructors first. source_positions_.Reset(nullptr); - typer_.Reset(nullptr); if (graph_zone_ == nullptr) return; // Destroy zone and clear pointers. graph_zone_scope_.Destroy(); @@ -291,8 +285,6 @@ class PipelineData { JSOperatorBuilder* javascript_; JSGraph* jsgraph_; JSTypeFeedbackTable* js_type_feedback_; - // TODO(dcarney): make this into a ZoneObject. - SmartPointer typer_; Schedule* schedule_; // All objects in the following group of fields are allocated in @@ -524,7 +516,11 @@ struct InliningPhase { struct TyperPhase { static const char* phase_name() { return "typer"; } - void Run(PipelineData* data, Zone* temp_zone) { data->typer()->Run(); } + void Run(PipelineData* data, Zone* temp_zone, Typer* typer) { + NodeVector roots(temp_zone); + data->jsgraph()->GetCachedNodes(&roots); + typer->Run(roots); + } }; @@ -1058,9 +1054,11 @@ Handle Pipeline::GenerateCode() { // Bailout here in case target architecture is not supported. if (!SupportedTarget()) return Handle::null(); + SmartPointer typer; if (info()->is_typing_enabled()) { // Type the graph. - Run(); + typer.Reset(new Typer(isolate(), data.graph(), info()->context())); + Run(typer.get()); RunPrintAndVerify("Typed"); } @@ -1073,7 +1071,7 @@ Handle Pipeline::GenerateCode() { if (FLAG_turbo_stress_loop_peeling) { Run(); - RunPrintAndVerify("Loop peeled", true); + RunPrintAndVerify("Loop peeled"); } if (info()->is_osr()) { @@ -1107,7 +1105,7 @@ Handle Pipeline::GenerateCode() { if (info()->is_osr()) { Run(); if (info()->bailout_reason() != kNoReason) return Handle::null(); - RunPrintAndVerify("OSR deconstruction"); + RunPrintAndVerify("OSR deconstruction", true); } } @@ -1124,6 +1122,9 @@ Handle Pipeline::GenerateCode() { data.source_positions()->RemoveDecorator(); + // Kill the Typer and thereby uninstall the decorator (if any). + typer.Reset(nullptr); + return ScheduleAndGenerateCode( Linkage::ComputeIncoming(data.instruction_zone(), info())); } diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index f7b3a1d..6311fd6 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -429,31 +429,14 @@ class Typer::Visitor : public Reducer { }; -void Typer::Run() { - { - // TODO(titzer): this is a hack. Reset types for interior nodes first. - NodeDeque deque(zone()); - NodeMarker marked(graph(), 2); - deque.push_front(graph()->end()); - marked.Set(graph()->end(), true); - while (!deque.empty()) { - Node* node = deque.front(); - deque.pop_front(); - // TODO(titzer): there shouldn't be a need to retype constants. - if (node->op()->ValueOutputCount() > 0) - NodeProperties::RemoveBounds(node); - for (Node* input : node->inputs()) { - if (!marked.Get(input)) { - marked.Set(input, true); - deque.push_back(input); - } - } - } - } +void Typer::Run() { Run(NodeVector(zone())); } + +void Typer::Run(const NodeVector& roots) { Visitor visitor(this); GraphReducer graph_reducer(zone(), graph()); graph_reducer.AddReducer(&visitor); + for (Node* const root : roots) graph_reducer.ReduceNode(root); graph_reducer.ReduceGraph(); } diff --git a/src/compiler/typer.h b/src/compiler/typer.h index 4c04ddb..66c4933 100644 --- a/src/compiler/typer.h +++ b/src/compiler/typer.h @@ -22,6 +22,8 @@ class Typer { ~Typer(); void Run(); + // TODO(bmeurer,jarin): Remove this once we have a notion of "roots" on Graph. + void Run(const ZoneVector& roots); Graph* graph() { return graph_; } MaybeHandle context() { return context_; } -- 2.7.4