From 2d07d762448c4a051186719e20c09cd89c034cde Mon Sep 17 00:00:00 2001 From: "jarin@chromium.org" Date: Mon, 3 Nov 2014 06:09:51 +0000 Subject: [PATCH] [turbofan] Do not use the generic graph algorithm for widening in the typer. This change uses an explicit queue for type-widening instead of the generic algorithm. The trouble with the generic algorithm was that it called the visitor on the same phi many times in a row (and thus caused unnecessary retyping). I also think that the queue-based fixpoint is more readable. The CL cuts running time of the nbody-java benchmark from ~19s to ~15s, the time spent in the typer goes from 4.5s to 1s. This is still a lot - the root cause appears to be slow handling of union subtyping (m*n for unions of sizes m and n). I see a re-typing of a single phi node taking > 100ms. I will work on a fix with Andreas, hopefully we can come up with some canonical representation of unions at least for the common cases (union of Smi constants). I have also changed the initial typer run to always compute a type, even if we already had a type for the node. This fixes one assert failure where context specialization updates a node without updating the type, which confuses the typer when widening (as some types suddenly narrow). BUG= R=bmeurer@chromium.org Review URL: https://codereview.chromium.org/689403002 Cr-Commit-Position: refs/heads/master@{#25053} git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25053 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/compiler/typer.cc | 75 ++++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 23 deletions(-) diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index 994a6384fd..6b32258d11 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -252,7 +252,7 @@ class Typer::RunVisitor : public Typer::Visitor { redo(NodeSet::key_compare(), NodeSet::allocator_type(typer->zone())) {} GenericGraphVisit::Control Post(Node* node) { - if (node->op()->ValueOutputCount() > 0 && !NodeProperties::IsTyped(node)) { + if (node->op()->ValueOutputCount() > 0) { Bounds bounds = TypeNode(node); NodeProperties::SetBounds(node, bounds); // Remember incompletely typed nodes for least fixpoint iteration. @@ -291,33 +291,64 @@ class Typer::NarrowVisitor : public Typer::Visitor { class Typer::WidenVisitor : public Typer::Visitor { public: - explicit WidenVisitor(Typer* typer) : Visitor(typer) {} + explicit WidenVisitor(Typer* typer) + : Visitor(typer), + local_zone_(zone()->isolate()), + enabled_(graph()->NodeCount(), true, &local_zone_), + queue_(&local_zone_) {} - GenericGraphVisit::Control Pre(Node* node) { - if (node->op()->ValueOutputCount() > 0) { - Bounds previous = BoundsOrNone(node); - Bounds current = TypeNode(node); + void Run(NodeSet* nodes) { + // Queue all the roots. + for (Node* node : *nodes) { + Queue(node); + } - // Speed up termination in the presence of range types: - current.upper = Weaken(current.upper, previous.upper); - current.lower = Weaken(current.lower, previous.lower); + while (!queue_.empty()) { + Node* node = queue_.front(); + queue_.pop(); - DCHECK(previous.lower->Is(current.lower)); - DCHECK(previous.upper->Is(current.upper)); + if (node->op()->ValueOutputCount() > 0) { + // Enable future queuing (and thus re-typing) of this node. + enabled_[node->id()] = true; - NodeProperties::SetBounds(node, current); - // Stop when nothing changed (but allow re-entry in case it does later). - return previous.Narrows(current) && current.Narrows(previous) - ? GenericGraphVisit::DEFER - : GenericGraphVisit::REENTER; - } else { - return GenericGraphVisit::SKIP; + // Compute the new type. + Bounds previous = BoundsOrNone(node); + Bounds current = TypeNode(node); + + // Speed up termination in the presence of range types: + current.upper = Weaken(current.upper, previous.upper); + current.lower = Weaken(current.lower, previous.lower); + + // Types should not get less precise. + DCHECK(previous.lower->Is(current.lower)); + DCHECK(previous.upper->Is(current.upper)); + + NodeProperties::SetBounds(node, current); + // If something changed, push all uses into the queue. + if (!(previous.Narrows(current) && current.Narrows(previous))) { + for (Node* use : node->uses()) { + Queue(use); + } + } + } + // If there is no value output, we deliberately leave the node disabled + // for queuing - there is no need to type it. } } - GenericGraphVisit::Control Post(Node* node) { - return GenericGraphVisit::REENTER; + void Queue(Node* node) { + // If the node is enabled for queuing, push it to the queue and disable it + // (to avoid queuing it multiple times). + if (enabled_[node->id()]) { + queue_.push(node); + enabled_[node->id()] = false; + } } + + private: + Zone local_zone_; + BoolVector enabled_; + ZoneQueue queue_; }; @@ -326,9 +357,7 @@ void Typer::Run() { graph_->VisitNodeInputsFromEnd(&typing); // Find least fixpoint. WidenVisitor widen(this); - for (NodeSetIter it = typing.redo.begin(); it != typing.redo.end(); ++it) { - graph_->VisitNodeUsesFrom(*it, &widen); - } + widen.Run(&typing.redo); }