On 05/06/2015 08:38 AM, Jason Ekstrand wrote:

On May 5, 2015 10:17 PM, "Tapani Pälli" <tapani.pa...@intel.com
<mailto:tapani.pa...@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 <kenn...@whitecape.org
<mailto:kenn...@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@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
 > http://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to