[Bug 90887] PhiMovesPass in register allocator broken - bugzilla-daemon-CC+yJ3UmIYqDUpFQwHEjaQ

From: bugzilla-daemon-CC+yJ3UmIYqDUpFQwHEjaQ@public.gmane.org
To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: [Bug 90887] PhiMovesPass in register allocator broken
Date: Sun, 28 Jun 2015 06:17:58 +0000	[thread overview]
Message-ID: <bug-90887-8800-oaNAxwKWzx@http.bugs.freedesktop.org/> (raw)
In-Reply-To: <bug-90887-8800-V0hAGp6uBxMKqLRl/0Ahz6D7qz1kEfGD2LY78lusg7I@public.gmane.org/>


[-- Attachment #1.1: Type: text/plain, Size: 2027 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=90887

--- Comment #14 from Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org> ---
OK, so among other changes, commit e43a3a66 had the following diff:

@@ -312,23 +337,26 @@ RegAlloc::PhiMovesPass::visit(BasicBlock *bb)
    Instruction *phi, *mov;
    BasicBlock *pb, *pn;

+   std::stack<BasicBlock *> stack;
+
    for (Graph::EdgeIterator ei = bb->cfg.incident(); !ei.end(); ei.next()) {
-      pb = pn = BasicBlock::get(ei.getNode());
+      pb = BasicBlock::get(ei.getNode());
       assert(pb);
-
-      if (needNewElseBlock(bb, pb)) {
-         pn = new BasicBlock(func);
-
-         // deletes an edge, iterator is invalid after this:
-         pb->cfg.detach(&bb->cfg);
-         pb->cfg.attach(&pn->cfg, Graph::Edge::TREE);
-         pn->cfg.attach(&bb->cfg, Graph::Edge::FORWARD); // XXX: check order !
-
-         assert(pb->getExit()->op != OP_CALL);
-         if (pb->getExit()->asFlow()->target.bb == bb)
-            pb->getExit()->asFlow()->target.bb = pn;
-         break;
-      }
+      if (needNewElseBlock(bb, pb))
+         stack.push(pb);
+   }
+   while (!stack.empty()) {
+      pb = stack.top();
+      pn = new BasicBlock(func);
+      stack.pop();
+
+      pb->cfg.detach(&bb->cfg);
+      pb->cfg.attach(&pn->cfg, Graph::Edge::TREE);
+      pn->cfg.attach(&bb->cfg, Graph::Edge::FORWARD);
+
+      assert(pb->getExit()->op != OP_CALL);
+      if (pb->getExit()->asFlow()->target.bb == bb)
+         pb->getExit()->asFlow()->target.bb = pn;
    }

    // insert MOVs (phi->src(j) should stem from j-th in-BB)

which the patch in attachment 116339 effectively undoes. [Why was that in
there? No idea. But presumably futzing with bb's affects needNewElseBlock's
decisions.] So I'm still tending towards my solution. But I'm not particularly
happy that it has to build up and use that hash table every time even if
nothing's changed.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 3140 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau
