On Tue, Jan 12, 2016 at 12:04 AM, Connor Abbott <[email protected]> wrote: > On Mon, Jan 11, 2016 at 11:44 PM, Ilia Mirkin <[email protected]> wrote: >> Actually this is still a little bogus... We'd have to ensure that >> *all* of the defs are used in this bb or later. I'll try to come up >> with something that still maintains some of the potential benefit but >> is correct. > > I don't know about the details of nv50, but I can say that we do a > pretty similar thing in NIR (removing phis whose sources are all the > same), and I don't see why it would be wrong. The SSA value must > dominate the phi node since it dominates all of its predecessors, so > it must dominate all uses of the phi, which means it should be ok to > replace uses of the phi with uses of the value. Of course, if the > source of the phi isn't an SSA value, then it won't work, but I > presume you don't have to handle that case here.
Not familiar enough with NIR details, but in nv50 ir, a single instruction may have multiple defs (like a texture instruction, which might return some or all of the rgba components). This GlobalCSE pass would find a phi node whose sources were to identical-resulted instructions (but potentially different instances of the same instruction, but in multiple BBs), and replace it with the first instruction. So you might end up in a situation where you have: BB1: a1, b1, c1, d1 = tex(x) use(b1, c1, d1) jump BB3 BB2: a2, b2, c2, d2 = tex(x) use(b2, c2, d2) jump BB3 BB3: a = phi(a1, a2) So here it would take the tex in bb1, delete it there, and just move it down to BB3 to replace the a = phi(). This obviously doesn't work. I either have to leave the old tex in place, or do some does-it-dominate checks for all the other args. Not sure what the best way to go is. -ilia _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
