[Mesa-dev] [PATCH] i965: Relax accumulator dependency scheduling on Gen < 6

[Iago Toral]

On Tue, 2014-05-06 at 11:45 -0700, Matt Turner wrote:
> Nice work.
> 
> On Tue, May 6, 2014 at 1:16 AM, Iago Toral Quiroga <itoral at igalia.com> wrote:
> > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > index 6e74803..37d3eab 100644
> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > @@ -676,6 +676,13 @@ backend_instruction::reads_accumulator_implicitly() const
> >  }
> >
> >  bool
> > +backend_instruction::writes_accumulator_implicitly(int gen) const
> > +{
> > +   return writes_accumulator ||
> > +          (gen < 6 && opcode >= BRW_OPCODE_ADD && opcode != BRW_OPCODE_NOP);
> 
> Since our virtual instruction opcodes are > BRW_OPCODE_NOP, they'll
> also be classified as writing the accumulator, whereas before they
> weren't.
> 
> I think the only ones (that are used on gen < 6) that generate
> hardware instructions that write the accumulator are
> 
>    FS_OPCODE_DDX
>    FS_OPCODE_DDY
>    FS_OPCODE_PIXEL_X
>    FS_OPCODE_PIXEL_Y
>    FS_OPCODE_CINTERP

After a quick look it looks like FS_OPCODE_CINTERP is implemented as a
MOV operation according to code in brw_fs_generator.cpp:

      case FS_OPCODE_CINTERP:
         brw_MOV(p, dst, src[0]);
         break;

so I guess this one will not write the accumulator in any case. If I am
missing something here, please let me know.

>    FS_OPCODE_LINTERP
> 
> If you update this function with these and it still passes piglit on
> gen < 6, then this patch is

I'll update the patch to include these (minus FS_OPCODE_CINTERP) and
check the piglit test again.

Thanks for the review!

Iago

> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>