[Mesa-dev] [PATCH 1/4] i965/vec4: Add basic common subexpression elimination.

Ian Romanick idr at freedesktop.org
Mon Jun 30 11:49:41 PDT 2014


On 06/25/2014 02:12 PM, Matt Turner wrote:
> From: Kenneth Graunke <kenneth at whitecape.org>
> 
> [mattst88]: Modified to perform CSE on instructions with
>             the same writemask. Offered no improvement before.
> 
> total instructions in shared programs: 1995633 -> 1995185 (-0.02%)
> instructions in affected programs:     14410 -> 13962 (-3.11%)
> 
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/Makefile.sources |   1 +
>  src/mesa/drivers/dri/i965/brw_vec4.cpp     |   1 +
>  src/mesa/drivers/dri/i965/brw_vec4.h       |   2 +
>  src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 237 +++++++++++++++++++++++++++++
>  4 files changed, 241 insertions(+)
>  create mode 100644 src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> 
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index 2570059..8f1d272 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -102,6 +102,7 @@ i965_FILES = \
>  	brw_util.c \
>  	brw_vec4.cpp \
>  	brw_vec4_copy_propagation.cpp \
> +	brw_vec4_cse.cpp \
>  	brw_vec4_generator.cpp \
>  	brw_vec4_gs.c \
>  	brw_vec4_gs_visitor.cpp \
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 24903f9..0d57399 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1747,6 +1747,7 @@ vec4_visitor::run()
>        progress = dead_control_flow_eliminate(this) || progress;
>        progress = opt_copy_propagation() || progress;
>        progress = opt_algebraic() || progress;
> +      progress = opt_cse() || progress;
>        progress = opt_register_coalesce() || progress;
>     } while (progress);
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 366198c..4a8eabb 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -426,6 +426,8 @@ public:
>     bool dead_code_eliminate();
>     bool virtual_grf_interferes(int a, int b);
>     bool opt_copy_propagation();
> +   bool opt_cse_local(bblock_t *, exec_list *);
> +   bool opt_cse();
>     bool opt_algebraic();
>     bool opt_register_coalesce();
>     void opt_set_dependency_control();
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> new file mode 100644
> index 0000000..33c7430
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> @@ -0,0 +1,237 @@
> +/*
> + * Copyright © 2012, 2013, 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "brw_vec4.h"
> +#include "brw_cfg.h"
> +
> +using namespace brw;
> +
> +/** @file brw_vec4_cse.cpp
> + *
> + * Support for local common subexpression elimination.
> + *
> + * See Muchnick's Advanced Compiler Design and Implementation, section
> + * 13.1 (p378).
> + */
> +
> +namespace {
> +struct aeb_entry : public exec_node {
> +   /** The instruction that generates the expression value. */
> +   vec4_instruction *generator;
> +
> +   /** The temporary where the value is stored. */
> +   src_reg tmp;
> +};
> +}
> +
> +static bool
> +is_expression(const vec4_instruction *const inst)

is_expression seems like a weird name for this.  It's not obvious to a
noob what this is doing.

> +{
> +   switch (inst->opcode) {
> +   case BRW_OPCODE_SEL:
> +   case BRW_OPCODE_NOT:
> +   case BRW_OPCODE_AND:
> +   case BRW_OPCODE_OR:
> +   case BRW_OPCODE_XOR:
> +   case BRW_OPCODE_SHR:
> +   case BRW_OPCODE_SHL:
> +   case BRW_OPCODE_ASR:
> +   case BRW_OPCODE_ADD:
> +   case BRW_OPCODE_MUL:
> +   case BRW_OPCODE_FRC:
> +   case BRW_OPCODE_RNDU:
> +   case BRW_OPCODE_RNDD:
> +   case BRW_OPCODE_RNDE:
> +   case BRW_OPCODE_RNDZ:
> +   case BRW_OPCODE_LINE:
> +   case BRW_OPCODE_PLN:
> +   case BRW_OPCODE_MAD:
> +   case BRW_OPCODE_LRP:
> +      return true;
> +   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 inst->mlen == 0;
> +   default:
> +      return false;
> +   }
> +}
> +
> +static bool
> +is_expression_commutative(enum opcode op)
> +{
> +   switch (op) {
> +   case BRW_OPCODE_AND:
> +   case BRW_OPCODE_OR:
> +   case BRW_OPCODE_XOR:
> +   case BRW_OPCODE_ADD:
> +   case BRW_OPCODE_MUL:
> +      return true;
> +   default:
> +      return false;
> +   }
> +}
> +
> +static bool
> +operands_match(enum opcode op, src_reg *xs, src_reg *ys)
> +{
> +   if (!is_expression_commutative(op)) {
> +      return xs[0].equals(ys[0]) && xs[1].equals(ys[1]) && xs[2].equals(ys[2]);
> +   } else {
> +      return (xs[0].equals(ys[0]) && xs[1].equals(ys[1])) ||
> +             (xs[1].equals(ys[0]) && xs[0].equals(ys[1]));
> +   }
> +}
> +
> +static bool
> +instructions_match(vec4_instruction *a, vec4_instruction *b)
> +{
> +   return a->opcode == b->opcode &&
> +          a->saturate == b->saturate &&
> +          a->conditional_mod == b->conditional_mod &&
> +          a->dst.type == b->dst.type &&
> +          a->dst.writemask == b->dst.writemask &&
> +          operands_match(a->opcode, a->src, b->src);
> +}
> +
> +bool
> +vec4_visitor::opt_cse_local(bblock_t *block, exec_list *aeb)
> +{
> +   bool progress = false;
> +
> +   void *cse_ctx = ralloc_context(NULL);
> +
> +   for (vec4_instruction *inst = (vec4_instruction *)block->start;
> +        inst != block->end->next;
> +        inst = (vec4_instruction *) inst->next) {

Are you planning to rebase this on your foreach_inst_in_block patch?

> +
> +      /* Skip some cases. */
> +      if (is_expression(inst) && !inst->predicate && inst->mlen == 0 &&

Doesn't is_expression already check inst->mlen == 0 for the cases that
count?

> +          !inst->conditional_mod)
> +      {

{ should go on previous line.

> +         bool found = false;
> +
> +         aeb_entry *entry;
> +         foreach_list(entry_node, aeb) {
> +            entry = (aeb_entry *) entry_node;
> +
> +            /* Match current instruction's expression against those in AEB. */
> +            if (instructions_match(inst, entry->generator)) {
> +               found = true;
> +               progress = true;
> +               break;
> +            }
> +         }
> +
> +         if (!found) {
> +            /* Our first sighting of this expression.  Create an entry. */
> +            aeb_entry *entry = ralloc(cse_ctx, aeb_entry);
> +            entry->tmp = src_reg(); /* file will be BAD_FILE */
> +            entry->generator = inst;
> +            aeb->push_tail(entry);
> +         } else {
> +            /* This is at least our second sighting of this expression.
> +             * If we don't have a temporary already, make one.
> +             */
> +            bool no_existing_temp = entry->tmp.file == BAD_FILE;
> +            if (no_existing_temp) {
> +               entry->tmp = src_reg(this, glsl_type::float_type);
> +               entry->tmp.type = inst->dst.type;
> +               entry->tmp.swizzle = BRW_SWIZZLE_XYZW;
> +
> +               vec4_instruction *copy = MOV(entry->generator->dst, entry->tmp);
> +               entry->generator->insert_after(copy);
> +               entry->generator->dst = dst_reg(entry->tmp);
> +            }
> +
> +            /* dest <- temp */
> +            assert(inst->dst.type == entry->tmp.type);
> +            vec4_instruction *copy = MOV(inst->dst, entry->tmp);
> +            copy->force_writemask_all = inst->force_writemask_all;
> +            inst->insert_before(copy);
> +
> +            /* Set our iterator so that next time through the loop inst->next
> +             * will get the instruction in the basic block after the one we've
> +             * removed.
> +             */
> +            vec4_instruction *prev = (vec4_instruction *)inst->prev;
> +
> +            inst->remove();
> +
> +            /* Appending an instruction may have changed our bblock end. */
> +            if (inst == block->end) {
> +               block->end = prev;
> +            }
> +
> +            inst = prev;
> +         }
> +      }
> +
> +      foreach_list_safe(entry_node, aeb) {
> +         aeb_entry *entry = (aeb_entry *)entry_node;
> +
> +         for (int i = 0; i < 3; i++) {
> +            /* Kill all AEB entries that use the destination we just
> +             * overwrote.
> +             */
> +            if (inst->dst.file == entry->generator->src[i].file &&
> +                inst->dst.reg == entry->generator->src[i].reg) {
> +               entry->remove();
> +               ralloc_free(entry);
> +               break;
> +            }
> +         }
> +      }
> +   }
> +
> +   ralloc_free(cse_ctx);
> +
> +   if (progress)
> +      invalidate_live_intervals();
> +
> +   return progress;
> +}
> +
> +bool
> +vec4_visitor::opt_cse()
> +{
> +   bool progress = false;
> +
> +   cfg_t cfg(&instructions);
> +
> +   for (int b = 0; b < cfg.num_blocks; b++) {
> +      bblock_t *block = cfg.blocks[b];
> +      exec_list aeb;
> +
> +      progress = opt_cse_local(block, &aeb) || progress;
> +   }
> +
> +   return progress;
> +}
> 



More information about the mesa-dev mailing list