[Mesa-dev] [RFC] i965: alternative to memctx for cleaning up nir variants

[Connor Abbott]

Why don't you just introduce a reference-counted wrapper around a
nir_shader * and pass that around instead? This seems like a
gallium-specific problem, so modifying other things to work around it
doesn't seem so appealing.

On Sat, Dec 19, 2015 at 8:55 PM, Rob Clark <robdclark at gmail.com> wrote:
> From: Rob Clark <robclark at freedesktop.org>
>
> Jason,
>
> How much do you hate this idea?  Seems like an easy alternative to
> using ralloc ctx's to clean up nir variants/clones, which would let
> us drop the parent memctx for nir_shader_create()/clone(), making
> it easier to introduce reference counting.
>
> ---
>  src/mesa/drivers/dri/i965/brw_compiler.h          | 22 ++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_fs.cpp              |  6 ++++--
>  src/mesa/drivers/dri/i965/brw_vec4.cpp            |  3 ++-
>  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp |  3 ++-
>  4 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h b/src/mesa/drivers/dri/i965/brw_compiler.h
> index c9e0317..eb1087a 100644
> --- a/src/mesa/drivers/dri/i965/brw_compiler.h
> +++ b/src/mesa/drivers/dri/i965/brw_compiler.h
> @@ -26,6 +26,7 @@
>  #include <stdio.h>
>  #include "brw_device_info.h"
>  #include "main/mtypes.h"
> +#include "nir.h"
>
>  #ifdef __cplusplus
>  extern "C" {
> @@ -651,6 +652,27 @@ struct brw_gs_prog_data
>
>  /** @} */
>
> +#ifdef __cplusplus
> +/* A common pattern is for brw_compile_xyz to create a clone of the original
> + * shader, modify it (various lowering passes, etc), and then throw it away.
> + * The scoped_clone helper class is just a way to use RAII to clean up the
> + * shader when it goes out of scope.
> + */
> +class scoped_clone
> +{
> +public:
> +   nir_shader *shader;
> +   scoped_clone(const nir_shader *s)
> +   {
> +      shader = nir_shader_clone(NULL, s);
> +   }
> +   ~scoped_clone()
> +   {
> +      ralloc_free(shader);
> +   }
> +};
> +#endif
> +
>  /**
>   * Compile a vertex shader.
>   *
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index c833ef0..44ea156 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -5490,7 +5490,8 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
>                 unsigned *final_assembly_size,
>                 char **error_str)
>  {
> -   nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> +   scoped_clone sc(src_shader);
> +   nir_shader *shader = sc.shader;
>     shader = brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex,
>                                        true);
>     shader = brw_postprocess_nir(shader, compiler->devinfo, true);
> @@ -5616,7 +5617,8 @@ brw_compile_cs(const struct brw_compiler *compiler, void *log_data,
>                 unsigned *final_assembly_size,
>                 char **error_str)
>  {
> -   nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> +   scoped_clone sc(src_shader);
> +   nir_shader *shader = sc.shader;
>     shader = brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex,
>                                        true);
>     shader = brw_postprocess_nir(shader, compiler->devinfo, true);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index a697bdf..a89e884 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1957,7 +1957,8 @@ brw_compile_vs(const struct brw_compiler *compiler, void *log_data,
>                 unsigned *final_assembly_size,
>                 char **error_str)
>  {
> -   nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> +   scoped_clone sc(src_shader);
> +   nir_shader *shader = sc.shader;
>     shader = brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex,
>                                        compiler->scalar_stage[MESA_SHADER_VERTEX]);
>     shader = brw_postprocess_nir(shader, compiler->devinfo,
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> index b13d36e..9ed7e0d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> @@ -617,7 +617,8 @@ brw_compile_gs(const struct brw_compiler *compiler, void *log_data,
>     memset(&c, 0, sizeof(c));
>     c.key = *key;
>
> -   nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
> +   scoped_clone sc(src_shader);
> +   nir_shader *shader = sc.shader;
>     shader = brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex,
>                                        compiler->scalar_stage[MESA_SHADER_GEOMETRY]);
>     shader = brw_postprocess_nir(shader, compiler->devinfo,
> --
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev