On Wed, Jun 02, 2010 at 11:39:57AM +0200, Nicolai Haehnle wrote: > On Wed, Jun 2, 2010 at 6:53 AM, Tom Stellard <tstel...@gmail.com> wrote: > > On Tue, Jun 01, 2010 at 12:00:16PM +0200, Nicolai Haehnle wrote: > >> On Tue, Jun 1, 2010 at 7:41 AM, tstellar <tstel...@gmail.com> wrote: > >> > From: Tom Stellard <tstel...@gmail.com> > >> > > >> > This fixes bug: > >> > > >> > https://bugs.freedesktop.org/show_bug.cgi?id=25109 > >> > >> Is this really correct? If I understand your patch correctly, what it > >> does is that TEX instructions that depend on earlier TEX instructions > >> will be emitted in the same TEX group on R300. So if you have > >> dependent texture reads like this: > >> > >> MOV r0, something; > >> TEX r1, r0, ...; > >> TEX r2, r1, ...; > >> > >> You will now emit both TEX in the same TEX indirection block, which I > >> thought was wrong, because the second TEX instruction will actually > >> use the contents of r1 *before* the first TEX instruction as > >> coordinates. At least that's how I thought the TEX hardware works: > >> > >> 1) Fetch all coordinates for all TEX instructions in an indirection block > >> 2) Execute all TEX instructions in parallel > >> 3) Store all results in the respective destination registers > >> > >> If my understanding is correct, then I believe your change miscompiles > >> the above shader fragment. Can you clarify this? > >> > > > > It looks like I am the one who misunderstood how TEX instructions work. > > You are right, the patch does miscompile your example. The shader > > I was having problems with looked like this: > > > > 10: TEX temp[13].xyz, temp[12].xy__, 2D[0]; > > 11: TEX temp[12].xyz, temp[11].xy__, 2D[0]; > > 12: TEX temp[11].xyz, temp[10].xy__, 2D[0]; > > 13: TEX temp[10].xyz, temp[9].xy__, 2D[0]; > > 14: TEX temp[9].xyz, temp[8].xy__, 2D[0]; > > 15: TEX temp[8].xyz, temp[7].xy__, 2D[0]; > > 16: TEX temp[7].xyz, input[4].xy__, 2D[0]; > > 17: TEX temp[6].xyz, temp[6].xy__, 2D[0]; > > 18: TEX temp[5].xyz, temp[5].xy__, 2D[0]; > > 19: TEX temp[4].xyz, temp[4].xy__, 2D[0]; > > 20: TEX temp[3].xyz, temp[3].xy__, 2D[0]; > > > > I think the bug is actually in the pair scheduler's dataflow analysis. > > Currently, the pair scheduler marks #10 as a dependency of #11, which > > would be OK if these were ALU instructions, but it shouldn't be a > > dependency for TEX instructions. > > Yes, I think what the dataflow analysis sees is that #10 reads > temp[12] while #11 overwrites temp[12], so its belief is that #10 must > be executed before #11. That's indeed a curious problem. > > So is this done after register allocation? Because it seems to me like > the register dealiasing should rename one of the occurences of > temp[12] in this example... >
Currently, the pair scheduler runs before the register allocation pass. I have attached the compiler output for this shader to the original bug if you want to take a look at it: https://bugs.freedesktop.org/show_bug.cgi?id=25109 -Tom _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
