[Mesa-dev] [PATCH 1/3] i965: Add reads_accumulator_implicitly() function.

[Eric Anholt]

Matt Turner <mattst88 at gmail.com> writes:

> ---
>  src/mesa/drivers/dri/i965/brw_shader.cpp | 16 ++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_shader.h   |  1 +
>  2 files changed, 17 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index f194437..c8796b3 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -664,6 +664,22 @@ backend_instruction::can_do_saturate() const
>  }
>  
>  bool
> +backend_instruction::reads_accumulator_implicitly() const
> +{
> +   switch (opcode) {
> +   case BRW_OPCODE_MAC:
> +   case BRW_OPCODE_MACH:
> +   /* FINISHME: Enable these if we ever start emitting them.
> +    * case BRW_OPCODE_SADA:
> +    * case BRW_OPCODE_SADA2:
> +    */

Let's just uncomment SADA2 right away to prevent pain in the future.
SAD2 doesn't read the acc, though.

Other than that, the first 2 patches are:

Reviewed-by: Eric Anholt <eric at anholt.net>

I think scheduling is still broken in the last one, because you're
removing the barrier deps on implicit-accumulator opcodes and replacing
them with explicit dependencies, but you're not tracking the accumulator
updates by almost-all-instructions pre-gen6.  The scheduler would be
free to slip in some unrelated instruction after the MUL in the
following snippet from brw_vec4_visitor.cpp:

            emit(MUL(acc, op[0], op[1]));
            emit(MACH(dst_null_d(), op[0], op[1]));
            emit(MOV(result_dst, src_reg(acc)));

(err, why are we doing MACH and MOV instead of just MACH into
result_dst?)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140409/78e7ca5e/attachment.sig>