[Mesa-dev] [PATCH 32/53] intel/fs: Mark LINTERP opcode as writing accumulator implicitly on pre-Gen7.

[Matt Turner]

On Thu, May 31, 2018 at 10:54 AM Francisco Jerez <currojerez at riseup.net> wrote:
>
> Matt Turner <mattst88 at gmail.com> writes:
>
> > On Fri, May 25, 2018 at 8:08 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> >> On May 25, 2018 15:28:22 Matt Turner <mattst88 at gmail.com> wrote:
> >>
> >>> On Thu, May 24, 2018 at 2:56 PM, Jason Ekstrand <jason at jlekstrand.net>
> >>> wrote:
> >>>>
> >>>> From: Francisco Jerez <currojerez at riseup.net>
> >>>>
> >>>> ---
> >>>> src/intel/compiler/brw_shader.cpp | 3 ++-
> >>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/src/intel/compiler/brw_shader.cpp
> >>>> b/src/intel/compiler/brw_shader.cpp
> >>>> index 141b64e..61211ef 100644
> >>>> --- a/src/intel/compiler/brw_shader.cpp
> >>>> +++ b/src/intel/compiler/brw_shader.cpp
> >>>> @@ -984,7 +984,8 @@
> >>>> backend_instruction::writes_accumulator_implicitly(const struct
> >>>> gen_device_info
> >>>> return writes_accumulator ||
> >>>>   (devinfo->gen < 6 &&
> >>>>    ((opcode >= BRW_OPCODE_ADD && opcode < BRW_OPCODE_NOP) ||
> >>>> -            (opcode >= FS_OPCODE_DDX_COARSE && opcode <=
> >>>> FS_OPCODE_LINTERP)));
> >>>> +            (opcode >= FS_OPCODE_DDX_COARSE && opcode <=
> >>>> FS_OPCODE_LINTERP))) ||
> >>>> +          (devinfo->gen < 7 && opcode == FS_OPCODE_LINTERP);
> >>>
> >>>
> >>> That's heavy-handed. Won't this prevent the scheduler from reordering
> >>> LINTERP instructions, even though we can only run into problems on
> >>> SIMD32?
> >>
> >>
> >> As long as none of them declare that they read it, re-ordering should be
> >> fine.
> >
> > The scheduler does
> >
> >       if (inst->writes_accumulator_implicitly(v->devinfo) &&
> >           !inst->dst.is_accumulator()) {
> >          add_dep(last_accumulator_write, n);
> >          last_accumulator_write = n;
> >       }
> >
> > I think that's going to cause the scheduler to be unable to reorder
> > any LINTERPs that implicitly write the accumulator.
>
> Which seems like the right thing to do, except where we can prove that
> the two accumulator writes are dead.  Do we care enough to take the
> trouble?  What's the shader-db damage on Gen4-6?

Fair enough. Let's punt.

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