(In reply to Ilia Mirkin from comment #24) > (In reply to jr from comment #23) > > (In reply to Ilia Mirkin from comment #22) > > > (In reply to jr from comment #21) > > > > Created attachment 117077 [details] [review] [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. 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