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

[Matt Turner]

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.