From d2861d682a235993844989f7742c9539c3e10245 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Wed, 2 Nov 2016 09:49:58 +1100 Subject: [PATCH] mesa/glsl: delete previously linked shaders earlier when linking MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This moves the delete linked shaders call to _mesa_clear_shader_program_data() which makes sure we delete them before returning due to any validation problems. It also reduces some code duplication. From the OpenGL 4.5 Core spec: "If LinkProgram failed, any information about a previous link of that program object is lost. Thus, a failed link does not restore the old state of program. ... If one of these commands is called with a program for which LinkProgram failed, no error is generated unless otherwise noted. Implementations may return information on variables and interface blocks that would have been active had the program been linked successfully. In cases where the link failed because the program required too many resources, these commands may help applications determine why limits were exceeded." Therefore it's expected that we shouldn't be able to query the program that failed to link and retrieve information about a previously successful link. Before this change the linker was doing validation before freeing the previously linked shaders and therefore could exit on failure before they were freed. This change also fixes an issue in compat profile where a program with no shaders attached is expect to fall back to fixed function but was instead trying to relink IR from a previous link. Reviewed-by: Tapani Pälli Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97715 Cc: "13.0" --- src/compiler/glsl/linker.cpp | 8 ------- src/compiler/glsl/standalone.cpp | 2 +- src/compiler/glsl/standalone_scaffolding.cpp | 10 ++++++++- src/compiler/glsl/standalone_scaffolding.h | 3 ++- src/mesa/main/shaderobj.c | 23 +++++++++----------- src/mesa/main/shaderobj.h | 3 ++- src/mesa/program/ir_to_mesa.cpp | 2 +- 7 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 03b866f2913..463c1f6c21f 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -4795,14 +4795,6 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) "type of shader\n"); } - for (unsigned int i = 0; i < MESA_SHADER_STAGES; i++) { - if (prog->_LinkedShaders[i] != NULL) { - _mesa_delete_linked_shader(ctx, prog->_LinkedShaders[i]); - } - - prog->_LinkedShaders[i] = NULL; - } - /* Link all shaders for a particular stage and validate the result. */ for (int stage = 0; stage < MESA_SHADER_STAGES; stage++) { diff --git a/src/compiler/glsl/standalone.cpp b/src/compiler/glsl/standalone.cpp index 055c433436a..f0964900938 100644 --- a/src/compiler/glsl/standalone.cpp +++ b/src/compiler/glsl/standalone.cpp @@ -421,7 +421,7 @@ standalone_compile_shader(const struct standalone_options *_options, } if ((status == EXIT_SUCCESS) && options->do_link) { - _mesa_clear_shader_program_data(whole_program); + _mesa_clear_shader_program_data(ctx, whole_program); link_shaders(ctx, whole_program); status = (whole_program->LinkStatus) ? EXIT_SUCCESS : EXIT_FAILURE; diff --git a/src/compiler/glsl/standalone_scaffolding.cpp b/src/compiler/glsl/standalone_scaffolding.cpp index 35e40d6c91b..d229368ed34 100644 --- a/src/compiler/glsl/standalone_scaffolding.cpp +++ b/src/compiler/glsl/standalone_scaffolding.cpp @@ -123,8 +123,16 @@ _mesa_delete_linked_shader(struct gl_context *ctx, } void -_mesa_clear_shader_program_data(struct gl_shader_program *shProg) +_mesa_clear_shader_program_data(struct gl_context *ctx, + struct gl_shader_program *shProg) { + for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { + if (shProg->_LinkedShaders[i] != NULL) { + _mesa_delete_linked_shader(ctx, shProg->_LinkedShaders[i]); + shProg->_LinkedShaders[i] = NULL; + } + } + shProg->NumUniformStorage = 0; shProg->UniformStorage = NULL; shProg->NumUniformRemapTable = 0; diff --git a/src/compiler/glsl/standalone_scaffolding.h b/src/compiler/glsl/standalone_scaffolding.h index b56dd3e8707..2c0454837a5 100644 --- a/src/compiler/glsl/standalone_scaffolding.h +++ b/src/compiler/glsl/standalone_scaffolding.h @@ -56,7 +56,8 @@ _mesa_delete_linked_shader(struct gl_context *ctx, struct gl_linked_shader *sh); extern "C" void -_mesa_clear_shader_program_data(struct gl_shader_program *); +_mesa_clear_shader_program_data(struct gl_context *ctx, + struct gl_shader_program *); extern "C" void _mesa_shader_debug(struct gl_context *ctx, GLenum type, GLuint *id, diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c index 136ac7b46a6..8fd574e5121 100644 --- a/src/mesa/main/shaderobj.c +++ b/src/mesa/main/shaderobj.c @@ -291,12 +291,18 @@ _mesa_new_shader_program(GLuint name) * Clear (free) the shader program state that gets produced by linking. */ void -_mesa_clear_shader_program_data(struct gl_shader_program *shProg) +_mesa_clear_shader_program_data(struct gl_context *ctx, + struct gl_shader_program *shProg) { - unsigned i; + for (gl_shader_stage sh = 0; sh < MESA_SHADER_STAGES; sh++) { + if (shProg->_LinkedShaders[sh] != NULL) { + _mesa_delete_linked_shader(ctx, shProg->_LinkedShaders[sh]); + shProg->_LinkedShaders[sh] = NULL; + } + } if (shProg->UniformStorage) { - for (i = 0; i < shProg->NumUniformStorage; ++i) + for (unsigned i = 0; i < shProg->NumUniformStorage; ++i) _mesa_uniform_detach_all_driver_storage(&shProg->UniformStorage[i]); ralloc_free(shProg->UniformStorage); shProg->NumUniformStorage = 0; @@ -347,11 +353,10 @@ _mesa_free_shader_program_data(struct gl_context *ctx, struct gl_shader_program *shProg) { GLuint i; - gl_shader_stage sh; assert(shProg->Type == GL_SHADER_PROGRAM_MESA); - _mesa_clear_shader_program_data(shProg); + _mesa_clear_shader_program_data(ctx, shProg); if (shProg->AttributeBindings) { string_to_uint_map_dtor(shProg->AttributeBindings); @@ -385,14 +390,6 @@ _mesa_free_shader_program_data(struct gl_context *ctx, shProg->TransformFeedback.VaryingNames = NULL; shProg->TransformFeedback.NumVarying = 0; - - for (sh = 0; sh < MESA_SHADER_STAGES; sh++) { - if (shProg->_LinkedShaders[sh] != NULL) { - _mesa_delete_linked_shader(ctx, shProg->_LinkedShaders[sh]); - shProg->_LinkedShaders[sh] = NULL; - } - } - free(shProg->Label); shProg->Label = NULL; } diff --git a/src/mesa/main/shaderobj.h b/src/mesa/main/shaderobj.h index 814a7f11552..12497328f98 100644 --- a/src/mesa/main/shaderobj.h +++ b/src/mesa/main/shaderobj.h @@ -99,7 +99,8 @@ extern struct gl_shader_program * _mesa_new_shader_program(GLuint name); extern void -_mesa_clear_shader_program_data(struct gl_shader_program *shProg); +_mesa_clear_shader_program_data(struct gl_context *ctx, + struct gl_shader_program *shProg); extern void _mesa_free_shader_program_data(struct gl_context *ctx, diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp index 5776d15c8a6..f4c2ad6770c 100644 --- a/src/mesa/program/ir_to_mesa.cpp +++ b/src/mesa/program/ir_to_mesa.cpp @@ -3049,7 +3049,7 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog) { unsigned int i; - _mesa_clear_shader_program_data(prog); + _mesa_clear_shader_program_data(ctx, prog); prog->LinkStatus = GL_TRUE; -- 2.30.2