[Mesa-dev] [PATCH 13/16] i965/fs: Add a saturation propagation optimization pass.

Pohjolainen, Topi topi.pohjolainen at intel.com
Thu Dec 26 05:54:30 PST 2013


On Thu, Dec 19, 2013 at 01:40:27PM -0800, Matt Turner wrote:
> Transforms, for example,
> 
> mul     vgrf3, vgrf2, vgrf1
> mov.sat vgrf4, vgrf3
> 
> into
> 
> mul.sat vgrf3, vgrf2, vgrf1
> mov     vgrf4, vgrf3
> 
> which gives register_coalescing an opportunity to remove the MOV
> instruction.
> 
> total instructions in shared programs: 1512588 -> 1501297 (-0.75%)
> instructions in affected programs:     723097 -> 711806 (-1.56%)
> GAINED:                                0
> LOST:                                  4
> ---
>  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                 |   1 +
>  .../dri/i965/brw_fs_saturate_propagation.cpp       | 102 +++++++++++++++++++++
>  4 files changed, 105 insertions(+)
>  create mode 100644 src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> 
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index 36d8261..3f901da 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -62,6 +62,7 @@ i965_FILES = \
>  	brw_fs_live_variables.cpp \
>  	brw_fs_peephole_predicated_break.cpp \
>  	brw_fs_reg_allocate.cpp \
> +	brw_fs_saturate_propagation.cpp \
>  	brw_fs_sel_peephole.cpp \
>  	brw_fs_value_numbering.cpp \
>  	brw_fs_vector_splitting.cpp \
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 08837da..12b6d4a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3304,6 +3304,7 @@ fs_visitor::run()
>           progress = opt_peephole_sel() || progress;
>           progress = dead_control_flow_eliminate(this) || progress;
>           vn_progress = opt_value_numbering();
> +         progress = opt_saturate_propagation() || progress;
>           progress = register_coalesce() || progress;
>  	 progress = compute_to_mrf() || progress;
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 75de18e..07360ae 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -368,6 +368,7 @@ public:
>     void try_replace_with_sel();
>     bool opt_peephole_sel();
>     bool opt_peephole_predicated_break();
> +   bool opt_saturate_propagation();
>     void emit_bool_to_cond_code(ir_rvalue *condition);
>     void emit_if_gen6(ir_if *ir);
>     void emit_unspill(fs_inst *inst, fs_reg reg, uint32_t spill_offset,
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> new file mode 100644
> index 0000000..91a1338
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> @@ -0,0 +1,102 @@
> +/*
> + * Copyright © 2013 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_live_variables.h"
> +#include "brw_cfg.h"
> +
> +/** @file brw_fs_saturate_propagation.cpp
> + */
> +
> +static bool
> +opt_saturate_propagation_local(fs_visitor *v, bblock_t *block)
> +{
> +   bool progress = false;
> +   int ip = block->start_ip;
> +
> +   for (fs_inst *inst = (fs_inst *)block->start;
> +        inst != block->end->next;
> +        inst = (fs_inst *) inst->next) {
> +      ip++;
> +
> +      if (inst->opcode != BRW_OPCODE_MOV ||
> +          inst->dst.file != GRF ||
> +          !inst->saturate)
> +         continue;
> +
> +      int src_var = v->live_intervals->var_from_reg(&inst->src[0]);
> +      int src_end_ip = v->live_intervals->end[src_var];
> +      if (src_end_ip > ip && !inst->dst.equals(inst->src[0]))
> +         continue;
> +
> +      int scan_ip = ip;
> +      bool interfered = false;
> +      for (fs_inst *scan_inst = (fs_inst *) inst->prev;
> +           scan_inst != block->start->prev;
> +           scan_inst = (fs_inst *) scan_inst->prev) {
> +         scan_ip--;
> +
> +         if (scan_inst->dst.file == GRF &&
> +             scan_inst->dst.reg == inst->src[0].reg &&
> +             scan_inst->dst.reg_offset == inst->src[0].reg_offset &&
> +             scan_inst->can_do_saturate()) {
> +            scan_inst->saturate = true;

Now once again trying to educate myself. If I'm reading this correctly it is
possible (at least in theory) to keep on setting multiple instructions as
saturated. Below there is the check for interference that controls if the loop
should be terminated before reaching the beginning of the block.

Would you have a simple example where an instruction is set saturated here but
where the 'interfered = true' assignment further down is not taken? Or is it
simply impossible and the check for interference is separate simply because it
is taking care of the case where the instruction writing the source of the
saturated mov-instuction cannot be set as saturated.

> +            inst->saturate = false;
> +            progress = true;
> +         }
> +	 for (int i = 0; i < 3; i++) {
> +            if (scan_inst->src[i].file == GRF &&
> +                scan_inst->src[i].reg == inst->src[0].reg &&
> +                scan_inst->src[i].reg_offset == inst->src[0].reg_offset) {
> +               interfered = true;
> +               break;
> +            }
> +         }
> +
> +         if (interfered)
> +            break;
> +      }
> +   }
> +
> +   return progress;
> +}
> +
> +bool
> +fs_visitor::opt_saturate_propagation()
> +{
> +   bool progress = false;
> +
> +   calculate_live_intervals();
> +
> +   cfg_t cfg(&instructions);
> +
> +   for (int b = 0; b < cfg.num_blocks; b++) {
> +      progress = opt_saturate_propagation_local(this, cfg.blocks[b])
> +                 || progress;
> +   }
> +
> +   if (progress)
> +      invalidate_live_intervals();
> +
> +   return progress;
> +}
> -- 
> 1.8.3.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list