[Mesa-dev] [PATCH 08/10] i965/fs: Allow immediates in MAD and LRP instructions.

[Kenneth Graunke]

On Wednesday, February 04, 2015 08:21:25 PM Matt Turner wrote:
> And then the opt_combine_constants() pass will pull them out into
> registers. This will allow us to do some algebraic optimizations on MAD
> and LRP.
> 
> total instructions in shared programs: 5946656 -> 5931320 (-0.26%)
> instructions in affected programs:     778247 -> 762911 (-1.97%)
> helped:                                3780
> HURT:                                  6
> GAINED:                                12
> LOST:                                  12
> ---
>  .../drivers/dri/i965/brw_fs_combine_constants.cpp  | 22 +++++++++++++++++++---
>  .../drivers/dri/i965/brw_fs_copy_propagation.cpp   |  6 ++++++
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
> index ad4a965..e9341b7 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
> @@ -60,6 +60,18 @@ could_coissue(const fs_inst *inst)
>     }
>  }
>  

/**
 * Returns true for instructions that don't support immediate sources.
 */
> +static bool
> +must_promote_imm(const fs_inst *inst)
> +{
> +   switch (inst->opcode) {
> +   case BRW_OPCODE_MAD:
> +   case BRW_OPCODE_LRP:
> +      return true;

Looking at fs_visitor::fix_math_operand, there are a few more cases we
could add in a follow up patch:

   case SHADER_OPCODE_RCP:
   case SHADER_OPCODE_RSQ:
   case SHADER_OPCODE_SQRT:
   case SHADER_OPCODE_EXP2:
   case SHADER_OPCODE_LOG2:
   case SHADER_OPCODE_POW:
   case SHADER_OPCODE_INT_QUOTIENT:
   case SHADER_OPCODE_INT_REMAINDER:
   case SHADER_OPCODE_SIN:
   case SHADER_OPCODE_COS:
      return brw->gen < 8;

and perhaps delete the copy propagation prohibitions and some of
fix_math_operand().  Just an idea :)

> +   default:
> +      return false;
> +   }
> +}
> +
>  struct reg_link {
>     DECLARE_RALLOC_CXX_OPERATORS(reg_link)
>  
> @@ -86,6 +98,7 @@ struct imm {
>     uint16_t reg;
>  
>     uint16_t uses_by_coissue;

  /**
   * Whether this constant is used by an instruction that can't handle an
   * immediate source (and already has to be promoted to a GRF).
   */

> +   bool must_promote;
>  
>     bool inserted;
>  
> @@ -153,12 +166,13 @@ fs_visitor::opt_combine_constants()
>     unsigned ip = -1;
>  
>     /* Make a pass through all instructions and count the number of times each
> -    * constant is used by coissueable instructions.
> +    * constant is used by coissueable instructions or instructions that cannot
> +    * take immediate arguments.
>      */
>     foreach_block_and_inst(block, fs_inst, inst, cfg) {
>        ip++;
>  
> -      if (!could_coissue(inst))
> +      if (!could_coissue(inst) && !must_promote_imm(inst))
>           continue;
>  
>        for (int i = 0; i < inst->sources; i++) {
> @@ -175,6 +189,7 @@ fs_visitor::opt_combine_constants()
>              imm->block = intersection;
>              imm->uses->push_tail(link(const_ctx, &inst->src[i]));
>              imm->uses_by_coissue += could_coissue(inst);
> +            imm->must_promote = imm->must_promote || must_promote_imm(inst);
>              imm->last_use_ip = ip;
>           } else {
>              imm = new_imm(&table, const_ctx);
> @@ -184,6 +199,7 @@ fs_visitor::opt_combine_constants()
>              imm->uses->push_tail(link(const_ctx, &inst->src[i]));
>              imm->val = val;
>              imm->uses_by_coissue = could_coissue(inst);
> +            imm->must_promote = must_promote_imm(inst);
>              imm->inserted = false;
>              imm->first_use_ip = ip;
>              imm->last_use_ip = ip;
> @@ -197,7 +213,7 @@ fs_visitor::opt_combine_constants()
>     for (int i = 0; i < table.len;) {
>        struct imm *imm = &table.imm[i];
>  
> -      if (imm->uses_by_coissue < 4) {
> +      if (!imm->must_promote && imm->uses_by_coissue < 4) {
>           table.imm[i] = table.imm[table.len - 1];
>           table.len--;
>           continue;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> index 68a266c..1b47851 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -574,6 +574,12 @@ fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry)
>           progress = true;
>           break;
>  
> +      case BRW_OPCODE_MAD:
> +      case BRW_OPCODE_LRP:
> +         inst->src[i] = val;
> +         progress = true;
> +         break;
> +
>        default:
>           break;
>        }
> 

LGTM.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150217/e0695690/attachment.sig>