On 04/02/2015 10:55 AM, Thomas Helland wrote: > 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"
There's a pretty significant bug triggered by that code. If you have a shader that sets uses a uniform initializer like uniform bool use_slow_method = false; then change that uniform via glUniform*, then trigger a recompile, the value will get reset to the in-shader value. It's quite a chain of events, but, IIRC, there was *one* piglit test that hit this. It shouldn't be too hard to fix. I think we'd just need a flag to the linker for it to ignore uniform initializers on the re-compile pass. > 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 <kenn...@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 <kenn...@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@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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev