[turbofan] Do not use the generic graph algorithm for widening in the typer.
authorjarin@chromium.org <jarin@chromium.org>
Mon, 3 Nov 2014 06:09:51 +0000 (06:09 +0000)
committerjarin@chromium.org <jarin@chromium.org>
Mon, 3 Nov 2014 06:10:18 +0000 (06:10 +0000)
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

index 994a638..6b32258 100644 (file)
@@ -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_) {}
+
+  void Run(NodeSet* nodes) {
+    // Queue all the roots.
+    for (Node* node : *nodes) {
+      Queue(node);
+    }
 
-  GenericGraphVisit::Control Pre(Node* node) {
-    if (node->op()->ValueOutputCount() > 0) {
-      Bounds previous = BoundsOrNone(node);
-      Bounds current = TypeNode(node);
+    while (!queue_.empty()) {
+      Node* node = queue_.front();
+      queue_.pop();
 
-      // Speed up termination in the presence of range types:
-      current.upper = Weaken(current.upper, previous.upper);
-      current.lower = Weaken(current.lower, previous.lower);
+      if (node->op()->ValueOutputCount() > 0) {
+        // Enable future queuing (and thus re-typing) of this node.
+        enabled_[node->id()] = true;
 
-      DCHECK(previous.lower->Is(current.lower));
-      DCHECK(previous.upper->Is(current.upper));
+        // Compute the new type.
+        Bounds previous = BoundsOrNone(node);
+        Bounds current = TypeNode(node);
 
-      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;
+        // 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<Node*> 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);
 }