<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - PhiMovesPass in register allocator broken"
href="https://bugs.freedesktop.org/show_bug.cgi?id=90887#c25">Comment # 25</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - PhiMovesPass in register allocator broken"
href="https://bugs.freedesktop.org/show_bug.cgi?id=90887">bug 90887</a>
from <span class="vcard"><a class="email" href="mailto:j-r@online.de" title="jr <j-r@online.de>"> <span class="fn">jr</span></a>
</span></b>
<pre>(In reply to Ilia Mirkin from <a href="show_bug.cgi?id=90887#c24">comment #24</a>)
<span class="quote">> (In reply to jr from <a href="show_bug.cgi?id=90887#c23">comment #23</a>)
> > (In reply to Ilia Mirkin from <a href="show_bug.cgi?id=90887#c22">comment #22</a>)
> > > (In reply to jr from <a href="show_bug.cgi?id=90887#c21">comment #21</a>)
> > > > Created <span class=""><a href="attachment.cgi?id=117077" name="attach_117077" title="Prototype of a more highlevel graph modification api">attachment 117077</a> <a href="attachment.cgi?id=117077&action=edit" title="Prototype of a more highlevel graph modification api">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=90887&attachment=117077'>[review]</a> [review] [review] [review] [review]
> > > > Prototype of a more highlevel graph modification api
> > > >
> > > > ...
> > >
> > > Hrm, I was about to (try to) push this out, but it doesn't seem to work. I
> > > made a few local adjustments, like
> > >
> > > RegAlloc::PhiMovesPass::isCriticalEdge(BasicBlock *b, BasicBlock *p)
> > > {
> > > return b->cfg.incidentCount() > 1 && p->cfg.outgoingCount() > 1;
> > > }
> > >
> > > ...
> > >
> > >
> > > where did BB:4 go? poof. not great :( this is with piglit's
> > > tests/shaders/ssa/fs-critical-edge.shader_test
> > >
> > > I guess I'll go with either your or my first patches. Want to get this fixed
> > > for Mesa 11, which is going to get branched off some time tomorrow.
> >
> > Sorry, most probably my prototype code is crap. I'll have an hour now to
> > look at it. I'll let you know if I find something. Otherwise I'm fine with
> > your first patch.
>
> Awesome! While I don't know whether the isCriticalEdge thing is the right
> thing to do instead of the current needNewElseBlock logic, I do think that
> splitEdge should be able to work properly even if needNewElseBlock always
> returns true.</span >
Yes, it should. My usual test cases plus the patch below look fine. My piglit
(from git://anongit.freedesktop.org/piglit) doesn't have a fs-critical-edge
test, though. Where do I find it?
That prototype patch was more of a RFC anyway to show how graph manipulation
could be made less intimidating. If you like the direction I will work some
more on it. But I think it is safe to go with your patch for now.
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
@@ -335,15 +335,7 @@ RegAlloc::BuildIntervalsPass::addLiveRange(Value *val,
bool
RegAlloc::PhiMovesPass::needNewElseBlock(BasicBlock *b, BasicBlock *p)
{
- if (b->cfg.incidentCount() <= 1)
- return false;
-
- int n = 0;
- for (Graph::EdgeIterator ei = p->cfg.outgoing(); !ei.end(); ei.next())
- if (ei.getType() == Graph::Edge::TREE ||
- ei.getType() == Graph::Edge::FORWARD)
- ++n;
- return (n == 2);
+ return (b->cfg.incidentCount() > 1) && (p->cfg.outgoingCount() > 1);
}
// For each operand of each PHI in b, generate a new value by inserting a MOV</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the QA Contact for the bug.</li>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>