[Mesa-dev] [PATCH 5/5] i965/fs: Debug the optimization passes by dumping instr to file.

[Kenneth Graunke]

On 04/11/2014 10:29 PM, Matt Turner wrote:
> With INTEL_DEBUG=optimizer, write the output of dump_instructions() to a
> file each time an optimization pass makes progress. This lets you easily
> diff successive files to see what an optimization pass did.
> 
> Example filenames written when running glxgears:
>    fs8-00-00-start
>    fs8-00-01-04-opt_copy_propagate
>    fs8-00-01-06-dead_code_eliminate
>    fs8-00-01-12-compute_to_mrf
>    fs8-00-02-06-dead_code_eliminate
>        |   |  |   |
>        |   |  |   `-- optimization pass name
>        |   |  |
>        |   |  `-- optimization pass number in the loop
>        |   |
>        |   `-- optimization loop interation
>        |
>        `-- shader program number
> 
> Note that with INTEL_DEBUG=optimizer, we disable compact_virtual_grfs,
> so that we can diff instruction lists across loop interations without
> the register numbers being changes.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 56 +++++++++++++++++++++++++++---------
>  1 file changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index c1d79e1..666cfa5b 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1704,6 +1704,9 @@ fs_visitor::split_virtual_grfs()
>  void
>  fs_visitor::compact_virtual_grfs()
>  {
> +   if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER))
> +      return;
> +
>     /* Mark which virtual GRFs are used, and count how many. */
>     int remap_table[this->virtual_grf_count];
>     memset(remap_table, -1, sizeof(remap_table));
> @@ -3258,25 +3261,52 @@ fs_visitor::run()
>  
>        opt_drop_redundant_mov_to_flags();
>  
> +#define OPT(pass, args...)                                                 \
> +   ({                                                                      \
> +      pass_num++;                                                          \
> +      bool progress = pass(args);                                          \
> +                                                                           \
> +      if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER) && progress) {           \
> +         char filename[64];                                                \
> +         snprintf(filename, 64, "fs%d-%02d-%02d-%02d-" #pass,              \
> +                  dispatch_width, shader_prog->Name, iteration, pass_num); \
> +                                                                           \
> +         backend_visitor::dump_instructions(filename);                                      \
> +      }                                                                    \
> +                                                                           \
> +      progress;                                                            \
> +   })

I like the use of the macro.  But...folding in the "progress = ... ||
progress" might be nice too.  It would also let us stick to standard C
here, rather than GNU extensions.

> +
> +      if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER)) {
> +         char filename[64];
> +         snprintf(filename, 64, "fs%d-%02d-00-start",
> +                  dispatch_width, shader_prog->Name);

I am really unexcited about fixed length buffers, although you did do
snprintf, and it'll probably not be a problem in practice.  Why not just
asprintf or ralloc_asprintf it and then free it?  It's no more code and
will just work.

> +
> +         backend_visitor::dump_instructions(filename);
> +      }
> +
>        bool progress;
> +      int iteration = 0;
>        do {
>  	 progress = false;
> +         iteration++;
> +         int pass_num = 0;
>  
>           compact_virtual_grfs();
>  
> -	 progress = remove_duplicate_mrf_writes() || progress;
> -
> -	 progress = opt_algebraic() || progress;
> -	 progress = opt_cse() || progress;
> -	 progress = opt_copy_propagate() || progress;
> -         progress = opt_peephole_predicated_break() || progress;
> -	 progress = dead_code_eliminate() || progress;
> -	 progress = dead_code_eliminate_local() || progress;
> -         progress = opt_peephole_sel() || progress;
> -         progress = dead_control_flow_eliminate(this) || progress;
> -         progress = opt_saturate_propagation() || progress;
> -         progress = register_coalesce() || progress;
> -	 progress = compute_to_mrf() || progress;
> +         progress = OPT(remove_duplicate_mrf_writes) || progress;
> +
> +         progress = OPT(opt_algebraic) || progress;
> +         progress = OPT(opt_cse) || progress;
> +         progress = OPT(opt_copy_propagate) || progress;
> +         progress = OPT(opt_peephole_predicated_break) || progress;
> +         progress = OPT(dead_code_eliminate) || progress;
> +         progress = OPT(dead_code_eliminate_local) || progress;
> +         progress = OPT(opt_peephole_sel) || progress;
> +         progress = OPT(dead_control_flow_eliminate, this) || progress;
> +         progress = OPT(opt_saturate_propagation) || progress;
> +         progress = OPT(register_coalesce) || progress;
> +         progress = OPT(compute_to_mrf) || progress;
>        } while (progress);
>  
>        lower_uniform_pull_constant_loads();
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140413/a3440d69/attachment.sig>