[Mesa-dev] [PATCH] i965: Delete most of the linked GLSL IR when using NIR.

[Tapani Pälli]

On 05/06/2015 08:38 AM, Jason Ekstrand wrote:
>
> On May 5, 2015 10:17 PM, "Tapani Pälli" <tapani.palli at intel.com
> <mailto:tapani.palli at intel.com>> wrote:
>  >
>  >
>  > On 05/06/2015 04:57 AM, Kenneth Graunke wrote:
>  >>
>  >> Vertex shader attribute and fragment shader output queries rely on being
>  >> able to inspect top-level ir_variable objects.  So, we have to keep
>  >> those.  However, functions and global temporary variables can be deleted
>  >> with impunity.
>  >>
>  >> Saves 58MB of memory when replaying a Dota 2 trace on Broadwell.
>  >>
>  >> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
> <mailto:kenneth at whitecape.org>>
>  >> ---
>  >>   src/mesa/drivers/dri/i965/brw_shader.cpp | 43
> +++++++++++++++++++++++++++++++-
>  >>   1 file changed, 42 insertions(+), 1 deletion(-)
>  >>
>  >> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
>  >> index c1fd859..6e4abb2 100644
>  >> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
>  >> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
>  >> @@ -138,6 +138,45 @@ brw_lower_packing_builtins(struct brw_context *brw,
>  >>      lower_packing_builtins(ir, ops);
>  >>   }
>  >>
>  >> +static bool
>  >> +is_vert_input_or_frag_output(gl_shader_stage stage, ir_variable *var)
>  >
>  >
>  > I believe you'll need to save any input and output independent of
> shader stage as with GL_ARB_program_interface_query application can
> query if some input or output is referenced by a given stage. Did you
> encounter any failures on Piglit tests?
>  >
>  > I've been working to get rid of ir_variable on the background but it
> has taken a bit of time as there seems to be always something else to be
> worked first 'priority wise' ... but I think this is a nice start
> anyways to get rid of most IR in memory.
>
> Do you have a branch somewhere with that work in it?  I would be
> interested in taking a look at it.  I probably won't really work on it
> but i would, at the very least, like to see where all the ir_variable's
> end up getting tied in.

Sorry not, it's just a simple hack patch ATM where I try to preserve 
only the ir_variable::data struct but I forgot that we really need a bit 
more (like name, type ..) so things fall apart. My plan next is first to 
introduce own structure where I keep adding fields as long as Piglit 
tests pass. I could probably use nir_variable as it's more minimal than 
ir_variable but that would be only for nir using drivers so I'm not sure 
if proceeding there makes sense right now (?)


> --Jason
>
>  >> +{
>  >> +   if (var) {
>  >> +      if (stage == MESA_SHADER_VERTEX) {
>  >> +         return var->data.mode == ir_var_shader_in ||
>  >> +                var->data.mode == ir_var_system_value;
>  >> +      }
>  >> +
>  >> +      if (stage == MESA_SHADER_FRAGMENT) {
>  >> +         return var->data.mode == ir_var_shader_out;
>  >> +      }
>  >> +   }
>  >> +   return false;
>  >> +}
>  >> +
>  >> +/**
>  >> + * Delete GLSL IR except for VS inputs and FS outputs.
>  >> + *
>  >> + * Once we've translated to NIR, we don't need most of the linked
> GLSL IR anymore.
>  >> + * However, GL API calls for introspecting certain shader
> inputs/outputs
>  >> + * (shader_query.cpp) require us to keep some top-level ir_variables.
>  >> + */
>  >> +static void
>  >> +delete_most_glsl_ir(struct gl_shader *shader)
>  >> +{
>  >> +   void *mem_ctx = ralloc_context(NULL);
>  >> +   ralloc_adopt(mem_ctx, shader->ir);
>  >> +
>  >> +   foreach_in_list_safe(ir_instruction, ir, shader->ir) {
>  >> +      if (!is_vert_input_or_frag_output(shader->Stage,
> ir->as_variable()))
>  >> +         ir->remove();
>  >> +   }
>  >> +
>  >> +   reparent_ir(shader->ir, shader->ir);
>  >> +   ralloc_free(mem_ctx);
>  >> +}
>  >> +
>  >> +
>  >>   static void
>  >>   process_glsl_ir(struct brw_context *brw,
>  >>                   struct gl_shader_program *shader_prog,
>  >> @@ -297,8 +336,10 @@ brw_link_shader(struct gl_context *ctx, struct
> gl_shader_program *shProg)
>  >>
>  >>         brw_add_texrect_params(prog);
>  >>
>  >> -      if (options->NirOptions)
>  >> +      if (options->NirOptions) {
>  >>            prog->nir = brw_create_nir(brw, shProg, prog,
> (gl_shader_stage) stage);
>  >> +         delete_most_glsl_ir(shader);
>  >> +      }
>  >>
>  >>         _mesa_reference_program(ctx, &prog, NULL);
>  >>      }
>  >>
>  > _______________________________________________
>  > mesa-dev mailing list
>  > mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>  > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>