[Mesa-dev] [PATCH 11/16] i965/fs: Add local value numbering optimization pass.

[Jordan Justen]

On Thu, Dec 19, 2013 at 1:40 PM, Matt Turner <mattst88 at gmail.com> wrote:
> total instructions in shared programs: 1520829 -> 1511991 (-0.58%)
> instructions in affected programs:     559725 -> 550887 (-1.58%)
> GAINED:                                6
> LOST:                                  9
> ---
>  src/mesa/drivers/dri/i965/Makefile.sources         |   1 +
>  src/mesa/drivers/dri/i965/brw_fs.cpp               |  37 +-
>  src/mesa/drivers/dri/i965/brw_fs.h                 |   1 +
>  .../drivers/dri/i965/brw_fs_value_numbering.cpp    | 398 +++++++++++++++++++++
>  4 files changed, 435 insertions(+), 2 deletions(-)
>  create mode 100644 src/mesa/drivers/dri/i965/brw_fs_value_numbering.cpp
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index 43f152e..36d8261 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -63,6 +63,7 @@ i965_FILES = \
>         brw_fs_peephole_predicated_break.cpp \
>         brw_fs_reg_allocate.cpp \
>         brw_fs_sel_peephole.cpp \
> +       brw_fs_value_numbering.cpp \
>         brw_fs_vector_splitting.cpp \
>         brw_fs_visitor.cpp \
>         brw_gs.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index e4a4737..08837da 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3285,9 +3285,11 @@ fs_visitor::run()
>        remove_dead_constants();
>        setup_pull_constants();
>
> -      bool progress;
> +      bool progress, cp_progress, vn_progress;
>        do {
>          progress = false;
> +         cp_progress = false;
> +         vn_progress = false;
>
>           compact_virtual_grfs();
>
> @@ -3295,16 +3297,47 @@ fs_visitor::run()
>
>          progress = opt_algebraic() || progress;
>          progress = opt_cse() || progress;
> -        progress = opt_copy_propagate() || progress;
>           progress = opt_peephole_predicated_break() || progress;
> +        cp_progress = opt_copy_propagate();

There is a tab here, according to:
git show mattst88/register-coalesce~5|grep -P "\+(\s*\t\s*)"

>          progress = dead_code_eliminate() || progress;
>          progress = dead_code_eliminate_local() || progress;
>           progress = opt_peephole_sel() || progress;
>           progress = dead_control_flow_eliminate(this) || progress;
> +         vn_progress = opt_value_numbering();

Could something like this help with the fighting, and remove the need
for the 'fixup' block below?
if (progress || !cp_progress)
   vn_progress = opt_value_numbering();

-Jordan

>           progress = register_coalesce() || progress;
>          progress = compute_to_mrf() || progress;
> +
> +         /* Value numbering rewrites
> +          *
> +          *    MOV g5, 1.0F
> +          *    MOV g6, 1.0F
> +          *
> +          * into
> +          *
> +          *    MOV g5, 1.0F
> +          *    MOV g6, g5
> +          *
> +          * but constant propagation undoes this. If these were the only two
> +          * passes to make progress, they're just fighting.
> +          */
> +         if (!progress && (cp_progress != vn_progress)) {
> +            progress = true;
> +         }
>        } while (progress);
>
> +      /* Copy propagation and value numbering were fighting, which means that
> +       * the last changes made by value numbering weren't useful, so rerun
> +       * copy propagation and the rest of the optimizations after it, modulo
> +       * value numbering.
> +       */
> +      if (cp_progress && vn_progress) {
> +         opt_copy_propagate();
> +         dead_code_eliminate();
> +         dead_code_eliminate_local();
> +         register_coalesce();
> +         compute_to_mrf();
> +      }
> +
>        lower_uniform_pull_constant_loads();
>
>        assign_curb_setup();