[Mesa-dev] [PATCH 3/3] i965: Add writes_accumulator flag

[Kenneth Graunke]

On 04/09/2014 01:47 PM, Matt Turner wrote:
> From: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> 
> Our hardware has an "accumulator" register, which can be used to store
> intermediate results across multiple instructions.  Many instructions
> can implicitly write a value to the accumulator in addition to their
> normal destination register.  This is enabled by the "AccWrEn" flag.
> 
> This patch introduces a new flag, inst->writes_accumulator, which
> allows us to express the AccWrEn notion in the IR.  It also creates a
> n ALU2_ACC macro to easily define emitters for instructions that
> implicitly write the accumulator.
> 
> Previously, we only supported implicit accumulator writes from the
> ADDC, SUBB, and MACH instructions.  We always enabled them on those
> instructions, and left them disabled for other instructions.
> 
> To take advantage of the MAC (multiply-accumulate) instruction, we
> need to be able to set AccWrEn on other types of instructions.
> 
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
> I split out is_accumulator() into a separate patch, and made some
> fixes to the scheduling code. Let me know if these changes look good
> to you, JP. (Patch formatted with -U15 as to see other sections of
> the scheduling code during review)
> 
>  src/mesa/drivers/dri/i965/brw_fs.cpp               | 26 ++++++----
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp     |  7 +--
>  .../drivers/dri/i965/brw_schedule_instructions.cpp | 58 ++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_shader.h             |  1 +
>  src/mesa/drivers/dri/i965/brw_vec4.cpp             | 15 ++----
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  7 +--
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp     | 17 +++++--
>  7 files changed, 95 insertions(+), 36 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index e576545..0eece60 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
[snip]
> @@ -2113,40 +2124,35 @@ fs_visitor::dead_code_eliminate()
>  
>           for (int i = 0; i < inst->regs_written; i++) {
>              int var = live_intervals->var_from_vgrf[inst->dst.reg];
>              assert(live_intervals->end[var + inst->dst.reg_offset + i] >= pc);
>              if (live_intervals->end[var + inst->dst.reg_offset + i] != pc) {
>                 dead = false;
>                 break;
>              }
>           }
>  
>           if (dead) {
>              /* Don't dead code eliminate instructions that write to the
>               * accumulator as a side-effect. Instead just set the destination
>               * to the null register to free it.
>               */
> -            switch (inst->opcode) {
> -            case BRW_OPCODE_ADDC:
> -            case BRW_OPCODE_SUBB:
> -            case BRW_OPCODE_MACH:
> +            if (inst->writes_accumulator) {
>                 inst->dst = fs_reg(retype(brw_null_reg(), inst->dst.type));
> -               break;

Pre-existing bug: we ought to set progress = true in this case.

> -            default:
> +            } else {
>                 inst->remove();
>                 progress = true;
> -               break;
>              }
>           }
>        }
>  
>        pc++;
>     }
>  
>     if (progress)
>        invalidate_live_intervals();
>  
>     return progress;
>  }
>  
>  struct dead_code_hash_key
>  {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140409/16923c31/attachment.sig>