This was supposed to go to the mailing list as well... sorry about that *sigh*
On Tue, Jun 1, 2010 at 7:41 AM, tstellar <[email protected]> wrote: > From: Tom Stellard <[email protected]> > > 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? cu, Nicolai > --- > .../dri/r300/compiler/radeon_pair_schedule.c | 39 > +++++++++++++------- > 1 files changed, 25 insertions(+), 14 deletions(-) > > diff --git a/src/mesa/drivers/dri/r300/compiler/radeon_pair_schedule.c > b/src/mesa/drivers/dri/r300/compiler/radeon_pair_schedule.c > index a279549..0641be5 100644 > --- a/src/mesa/drivers/dri/r300/compiler/radeon_pair_schedule.c > +++ b/src/mesa/drivers/dri/r300/compiler/radeon_pair_schedule.c > @@ -208,21 +208,32 @@ static void emit_all_tex(struct schedule_state * s, > struct rc_instruction * befo > > assert(s->ReadyTEX); > > - /* Don't let the ready list change under us! */ > - readytex = s->ReadyTEX; > - s->ReadyTEX = 0; > - > - /* Node marker for R300 */ > - struct rc_instruction * inst_begin = rc_insert_new_instruction(s->C, > before->Prev); > - inst_begin->U.I.Opcode = RC_OPCODE_BEGIN_TEX; > - > - /* Link texture instructions back in */ > - while(readytex) { > - struct schedule_instruction * tex = readytex; > - readytex = readytex->NextReady; > + if(s->ReadyTEX){ > + /* Node marker for R300 */ > + struct rc_instruction * inst_begin = > + rc_insert_new_instruction(s->C, before->Prev); > + inst_begin->U.I.Opcode = RC_OPCODE_BEGIN_TEX; > + } > > - rc_insert_instruction(before->Prev, tex->Instruction); > - commit_instruction(s, tex); > + /* The purpose of this outer loop is to emit TEX instruction that > + * have become ready as a result of the inner loop. This prevents > + * the fragmentation of TEX blocks, which for some shaders (in Civ4 > + * for example) was creating too many texture indirections on r300 > + * cards. > + */ > + while(s->ReadyTEX){ > + /* Don't let the ready list change under us! */ > + readytex = s->ReadyTEX; > + s->ReadyTEX = 0; > + > + /* Link texture instructions back in */ > + while(readytex) { > + struct schedule_instruction * tex = readytex; > + readytex = readytex->NextReady; > + > + rc_insert_instruction(before->Prev, tex->Instruction); > + commit_instruction(s, tex); > + } > } > } > > -- > 1.6.4.4 _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
