[Mesa-dev] [PATCH] i965: Free dead GLSL IR one last time.

[Thomas Helland]

This reminds me of a patch Eric wrote that probably
fell through the cracks when he migrated jobs:

https://freedesktop.org/patch/26778/
"Free the compiled shader IR after it has been linked"

It got an R-B, but seems like it never made it to upstream.
I don't know if it still applies to the way things are now though.
There might have been a discussion on IRC that concluded
that it should be dropped? I don't know.
It still has status "New" in patchwork, so seems unlikely.
Seems like it should be sort of a big deal, so thought I'd ping it.

Regards,
Thomas

2015-04-02 11:04 GMT+02:00 Kenneth Graunke <kenneth at whitecape.org>:
> While working on NIR's memory allocation model, I realized the GLSL IR
> memory model was broken.
>
> During glCompileShader, we allocate everything out of the
> _mesa_glsl_parse_state context, and reparent it to gl_shader at the end.
>
> During glLinkProgram, we allocate everything out of a temporary context,
> then reparent it to the exec_list containing the linked IR.
>
> But during brw_link_shader - the driver's final opportunity to do
> lowering and optimization - we just allocated everything out of the
> permanent context given to us by the linker.  That memory stayed
> forever.
>
> Notably, passes like brw_fs_channel_expressions cause us to churn the
> majority of the code, so we really want to free dead IR here.
>
> Saves 125MB of memory when replaying a Dota 2 trace on Broadwell.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_shader.cpp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> This depends on my new ralloc_adopt() API, proposed here:
> http://lists.freedesktop.org/archives/mesa-dev/2015-March/080763.html
>
> XXX: need to confirm piglit results (I kicked off tests, but am waiting for results)
> XXX: should probably Cc stable.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 0dda9bb..b923e54 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -144,6 +144,11 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
>
>        _mesa_copy_linked_program_data((gl_shader_stage) stage, shProg, prog);
>
> +      /* Temporary memory context for any new IR. */
> +      void *mem_ctx = ralloc_context(NULL);
> +
> +      ralloc_adopt(mem_ctx, shader->base.ir);
> +
>        bool progress;
>
>        /* lower_packing_builtins() inserts arithmetic instructions, so it
> @@ -249,6 +254,13 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
>
>        _mesa_reference_program(ctx, &prog, NULL);
>
> +      /* Now that we've finished altering the linked IR, reparent any live IR back
> +       * to the permanent memory context, and free the temporary one (discarding any
> +       * junk we optimized away).
> +       */
> +      reparent_ir(shader->base.ir, shader->base.ir);
> +      ralloc_free(mem_ctx);
> +
>        if (ctx->_Shader->Flags & GLSL_DUMP) {
>           fprintf(stderr, "\n");
>           fprintf(stderr, "GLSL IR for linked %s program %d:\n",
> --
> 2.3.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev