[Mesa-dev] [PATCH v2 32/53] intel/fs: Mark LINTERP opcode as writing accumulator on platforms without PLN

[Francisco Jerez]

Jason Ekstrand <jason at jlekstrand.net> writes:

> From: Francisco Jerez <currojerez at riseup.net>
>
> When we don't have PLN (gen4 and gen11+), we implement LINTERP as either
> LINE+MAC or a pair of MADs.  In both cases, the accumulator is written
> by the first of the two instructions and read by the second.  Even
> though the accumulator value isn't actually ever used from a logical
> instruction perspective, it is trashed so we need to make the scheduler
> aware.  Otherwise, the scheduler could end up re-ordering instructions
> and putting a LINTERP between another an instruction which writes the
> accumulator and another which tries to use that result.
>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  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..dfd2c5c 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))) ||
> +          (opcode == FS_OPCODE_LINTERP && !devinfo->has_pln);

The devinfo->has_pln condition is technically inaccurate, because even
SNB will fall back to the non-PLN path which overwrites the accumulator
for certain valid IR, which yeah I'm aware is not *typically*
encountered before this series, but why make things more inaccurate than
the original only to revert back to a devinfo->gen based check in
PATCHv2 33?

I think I'd squash the last hunk of PATCHv2 33 into this one which would
give you something functionally equivalent to v1 but updated to handle
Gen11 correctly.

>  }
>  
>  bool
> -- 
> 2.5.0.400.gff86faf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180526/5d50e871/attachment.sig>