[Mesa-dev] [PATCH] nv50/ir: replace the right def in global cse

[Ilia Mirkin]

On Tue, Jan 12, 2016 at 12:04 AM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> On Mon, Jan 11, 2016 at 11:44 PM, Ilia Mirkin <imirkin at alum.mit.edu> 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