[Mesa-dev] [PATCH 2/2] i965/fs: Add a local common subexpression elimination pass.

Ian Romanick idr at freedesktop.org
Fri May 11 14:27:28 PDT 2012


On 05/10/2012 04:10 PM, Kenneth Graunke wrote:
> Total instructions: 18210 ->  17836
> 49/163 programs affected (30.1%)
> 12888 ->  12514 instructions in affected programs (2.9% reduction)
>
> This reduces Lightsmark's "Scale down filter" shader from 395
> instructions to 283, a whopping 28%.  It also reduces register pressure
> significantly: the SIMD8 program now uses 29 registers instead of 101,
> giving us more than enough room for a SIMD16 program.
>
> Signed-off-by: Kenneth Graunke<kenneth at whitecape.org>

This looks like a pretty straight forward implementation of the 
algorithm from the book.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> ---
>   src/mesa/drivers/dri/i965/Makefile.sources |    1 +
>   src/mesa/drivers/dri/i965/brw_fs.cpp       |    1 +
>   src/mesa/drivers/dri/i965/brw_fs.h         |    4 +
>   src/mesa/drivers/dri/i965/brw_fs_cse.cpp   |  194 ++++++++++++++++++++++++++++
>   4 files changed, 200 insertions(+)
>   create mode 100644 src/mesa/drivers/dri/i965/brw_fs_cse.cpp
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index c564d95..838e1d3 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -115,6 +115,7 @@ i965_CXX_FILES = \
>   	brw_cubemap_normalize.cpp \
>   	brw_fs.cpp \
>   	brw_fs_cfg.cpp \
> +	brw_fs_cse.cpp \
>   	brw_fs_emit.cpp \
>   	brw_fs_live_variables.cpp \
>   	brw_fs_visitor.cpp \
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index bcdedf0..47dfec2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1681,6 +1681,7 @@ fs_visitor::run()
>
>   	 progress = propagate_constants() || progress;
>   	 progress = opt_algebraic() || progress;
> +	 progress = opt_cse() || progress;
>   	 progress = register_coalesce() || progress;
>   	 progress = compute_to_mrf() || progress;
>   	 progress = dead_code_eliminate() || progress;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index d4473a2..0a9b8aa 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -49,6 +49,8 @@ extern "C" {
>   #include "glsl/glsl_types.h"
>   #include "glsl/ir.h"
>
> +class fs_bblock;
> +
>   enum register_file {
>      BAD_FILE,
>      ARF,
> @@ -507,6 +509,8 @@ public:
>      void calculate_live_intervals();
>      bool propagate_constants();
>      bool opt_algebraic();
> +   bool opt_cse();
> +   bool opt_cse_local(fs_bblock *block, exec_list *aeb);
>      bool register_coalesce();
>      bool compute_to_mrf();
>      bool dead_code_eliminate();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> new file mode 100644
> index 0000000..d7cab0b
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> @@ -0,0 +1,194 @@
> +/*
> + * Copyright © 2012 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_fs.h"
> +#include "brw_fs_cfg.h"
> +
> +/** @file brw_fs_cse.cpp
> + *
> + * Support for local common subexpression elimination.
> + *
> + * See Muchnik's Advanced Compiler Design and Implementation, section
> + * 13.1 (p378).
> + */
> +
> +namespace {
> +struct aeb_entry : public exec_node {
> +   /** The instruction that generates the expression value. */
> +   fs_inst *generator;
> +
> +   /** The temporary where the value is stored. */
> +   fs_reg tmp;
> +};
> +}
> +
> +static bool
> +is_expression(const fs_inst *const inst)
> +{
> +   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_RSR:
> +   case BRW_OPCODE_RSL:
> +   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 FS_OPCODE_CINTERP:
> +   case FS_OPCODE_LINTERP:
> +      return true;
> +   default:
> +      return false;
> +   }
> +}
> +
> +static bool
> +operands_match(fs_reg *xs, fs_reg *ys)
> +{
> +   return xs[0].equals(ys[0])&&  xs[1].equals(ys[1])&&  xs[2].equals(ys[2]);
> +}
> +
> +bool
> +fs_visitor::opt_cse_local(fs_bblock *block, exec_list *aeb)
> +{
> +   bool progress = false;
> +
> +   void *mem_ctx = ralloc_context(this->mem_ctx);
> +
> +   for (fs_inst *inst = block->start;
> +	inst != block->end->next;
> +	inst = (fs_inst *) inst->next) {
> +
> +      /* Skip some cases. */
> +      if (is_expression(inst)&&  !inst->predicated&&  inst->mlen == 0&&
> +          !inst->force_uncompressed&&  !inst->force_sechalf)
> +      {
> +	 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 (inst->opcode == entry->generator->opcode&&
> +		inst->saturate == entry->generator->saturate&&
> +		operands_match(entry->generator->src, inst->src)) {
> +
> +	       found = true;
> +	       progress = true;
> +	       break;
> +	    }
> +	 }
> +
> +	 if (!found) {
> +	    /* Our first sighting of this expression.  Create an entry. */
> +	    aeb_entry *entry = ralloc(mem_ctx, aeb_entry);
> +	    entry->tmp = reg_undef;
> +	    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 = fs_reg(this, glsl_type::float_type);
> +	       entry->tmp.type = inst->dst.type;
> +
> +	       fs_inst *copy = new(ralloc_parent(inst))
> +		  fs_inst(BRW_OPCODE_MOV, entry->generator->dst, entry->tmp);
> +	       entry->generator->insert_after(copy);
> +	       entry->generator->dst = entry->tmp;
> +	    }
> +
> +	    /* dest<- temp */
> +	    fs_inst *copy = new(ralloc_parent(inst))
> +	       fs_inst(BRW_OPCODE_MOV, inst->dst, entry->tmp);
> +	    inst->replace_with(copy);
> +
> +	    /* Appending an instruction may have changed our bblock end. */
> +	    if (inst == block->end) {
> +	       block->end = copy;
> +	    }
> +
> +	    /* Continue iteration with copy->next */
> +	    inst = copy;
> +	 }
> +      }
> +
> +      /* Kill all AEB entries that use the destination. */
> +      int start_offset = inst->dst.reg_offset;
> +      int end_offset = start_offset + inst->regs_written();
> +
> +      foreach_list_safe(entry_node, aeb) {
> +	 aeb_entry *entry = (aeb_entry *)entry_node;
> +
> +	 for (int i = 0; i<  3; i++) {
> +	    if (entry->generator->src[i].file == inst->dst.file&&
> +		entry->generator->src[i].reg == inst->dst.reg&&
> +		entry->generator->src[i].reg_offset>= start_offset&&
> +		entry->generator->src[i].reg_offset<  end_offset) {
> +	       entry->remove();
> +	       ralloc_free(entry);
> +	       break;
> +	    }
> +	 }
> +      }
> +   }
> +
> +   ralloc_free(mem_ctx);
> +
> +   if (progress)
> +      this->live_intervals_valid = false;
> +
> +   return progress;
> +}
> +
> +bool
> +fs_visitor::opt_cse()
> +{
> +   bool progress = false;
> +
> +   fs_cfg cfg(this);
> +
> +   for (int b = 0; b<  cfg.num_blocks; b++) {
> +      fs_bblock *block = cfg.blocks[b];
> +      exec_list aeb;
> +
> +      progress = opt_cse_local(block,&aeb) || progress;
> +   }
> +
> +   return progress;
> +}



More information about the mesa-dev mailing list