[Mesa-dev] [PATCH v3 1/1] i965: Do not overwrite optimizer dumps

[Jason Ekstrand]

On Nov 27, 2015 3:55 AM, "Juan A. Suarez Romero" <jasuarez at igalia.com>
wrote:
>
> When using INTEL_DEBUG=optimizer, each optimizing step is dump to disk,
> in a separate file.
>
> But as fs_visitor::optimize() and vec4_visitor::run() are called more
> than once, it ends up overwriting the files already on disk, loosing
> then previous optimizer steps.
>
> To avoid this, add a new static variable that tracks the global
> iteration across the entire life of the program running.
>
> v2: use atomic_inc() for the static variable (Jason).
>
> v3: define local variable as const (Jason).
>
> Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp   | 13 +++++++++----
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 13 ++++++++-----
>  2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 7904f4d..eee9e1d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -39,6 +39,7 @@
>  #include "brw_program.h"
>  #include "brw_dead_control_flow.h"
>  #include "glsl/nir/glsl_types.h"
> +#include "util/u_atomic.h"
>
>  using namespace brw;
>
> @@ -4948,6 +4949,9 @@ fs_visitor::calculate_register_pressure()
>  void
>  fs_visitor::optimize()
>  {
> +   static int global_iteration_atomic = 0;
> +   const int global_iteration =
p_atomic_inc_return(&global_iteration_atomic);
> +
>     /* Start by validating the shader we currently have. */
>     validate();
>
> @@ -4978,8 +4982,9 @@ fs_visitor::optimize()
>                                                                          \
>        if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER) && this_progress) {   \
>           char filename[64];                                             \
> -         snprintf(filename, 64, "%s%d-%s-%02d-%02d-" #pass,
\
> -                  stage_abbrev, dispatch_width, nir->info.name,
iteration, pass_num); \
> +         snprintf(filename, 64, "%s%d-%s-%02d-%02d-%02d-" #pass,        \
> +                  stage_abbrev, dispatch_width, nir->info.name,         \
> +                  global_iteration, iteration, pass_num);               \
>                                                                          \
>           backend_shader::dump_instructions(filename);                   \
>        }                                                                 \
> @@ -4992,8 +4997,8 @@ fs_visitor::optimize()
>
>     if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER)) {
>        char filename[64];
> -      snprintf(filename, 64, "%s%d-%s-00-start",
> -               stage_abbrev, dispatch_width, nir->info.name);
> +      snprintf(filename, 64, "%s%d-%s-%02d-00-00-start",
> +               stage_abbrev, dispatch_width, nir->info.name,
global_iteration);
>
>        backend_shader::dump_instructions(filename);
>     }
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 9a79d67..7909f1d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -29,6 +29,7 @@
>  #include "brw_vec4_live_variables.h"
>  #include "brw_dead_control_flow.h"
>  #include "program/prog_parameter.h"
> +#include "util/u_atomic.h"
>
>  #define MAX_INSTRUCTION (1 << 30)
>
> @@ -1779,6 +1780,9 @@ vec4_visitor::convert_to_hw_regs()
>  bool
>  vec4_visitor::run()
>  {
> +   static int global_iteration_atomic = 0;
> +   const int global_iteration =
p_atomic_inc_return(&global_iteration_atomic);
> +
>     if (shader_time_index >= 0)
>        emit_shader_time_begin();
>
> @@ -1812,8 +1816,8 @@ vec4_visitor::run()
>                                                                         \
>        if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER) && this_progress) {  \
>           char filename[64];                                            \
> -         snprintf(filename, 64, "%s-%s-%02d-%02d-" #pass,              \
> -                  stage_abbrev, nir->info.name, iteration, pass_num);  \
> +         snprintf(filename, 64, "%s-%s-%02d-%02d-%02d-" #pass,         \
> +                  stage_abbrev, nir->info.name, global_iteration,
iteration, pass_num); \
>                                                                         \
>           backend_shader::dump_instructions(filename);                  \
>        }                                                                \
> @@ -1822,11 +1826,10 @@ vec4_visitor::run()
>        this_progress;                                                   \
>     })
>
> -

Unneeded white space change.

Other than that, this looks much better.

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

I'd like to get Matt's OK before pushing anything though.  He may yet have
a better idea about how to make things properly unique.

>     if (unlikely(INTEL_DEBUG & DEBUG_OPTIMIZER)) {
>        char filename[64];
> -      snprintf(filename, 64, "%s-%s-00-start",
> -               stage_abbrev, nir->info.name);
> +      snprintf(filename, 64, "%s-%s-%02d-00-00-start",
> +               stage_abbrev, nir->info.name, global_iteration);
>
>        backend_shader::dump_instructions(filename);
>     }
> --
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151127/c3a77db7/attachment.html>