From mboxrd@z Thu Jan 1 00:00:00 1970
From: bugzilla-daemon-CC+yJ3UmIYqDUpFQwHEjaQ@public.gmane.org
Subject: [Bug 90887] PhiMovesPass in register allocator broken
Date: Sun, 28 Jun 2015 06:17:58 +0000
Message-ID:
References:
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="===============0375236360=="
Return-path:
In-Reply-To:
List-Unsubscribe: ,
List-Archive:
List-Post:
List-Help:
List-Subscribe: ,
Errors-To: nouveau-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Sender: "Nouveau"
To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
List-Id: nouveau.vger.kernel.org
--===============0375236360==
Content-Type: multipart/alternative; boundary="1435472277.225BfD0F1.21193"; charset="UTF-8"
--1435472277.225BfD0F1.21193
Date: Sun, 28 Jun 2015 06:17:57 +0000
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
https://bugs.freedesktop.org/show_bug.cgi?id=90887
--- Comment #14 from Ilia Mirkin ---
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 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.
--1435472277.225BfD0F1.21193
Date: Sun, 28 Jun 2015 06:17:57 +0000
MIME-Version: 1.0
Content-Type: text/html; charset="UTF-8"
Comment # 14
on bug 90887
from Ilia Mirkin
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 [details] 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.
--1435472277.225BfD0F1.21193--
--===============0375236360==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Disposition: inline
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KTm91dmVhdSBt
YWlsaW5nIGxpc3QKTm91dmVhdUBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZy
ZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL25vdXZlYXUK
--===============0375236360==--