On 11/02/2015 09:16 AM, Ilia Mirkin wrote:
On Mon, Nov 2, 2015 at 1:58 AM, Tapani Pälli <tapani.pa...@intel.com> wrote:Patch changes linker to allocate gl_shader_variable instead of using ir_variable. This makes it possible to get rid of ir_variables and ir in memory after linking. v2: check that we do not create duplicate entries with packed varyings Signed-off-by: Tapani Pälli <tapani.pa...@intel.com> --- src/glsl/linker.cpp | 58 +++++++++++++++++++++++++++++++++++------- src/mesa/main/mtypes.h | 56 ++++++++++++++++++++++++++++++++++++++++ src/mesa/main/shader_query.cpp | 36 +++++++++++++------------- 3 files changed, 123 insertions(+), 27 deletions(-) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 48dd2d3..d0353b4 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -3341,6 +3341,27 @@ build_stageref(struct gl_shader_program *shProg, const char *name, return stages; } +/** + * Create gl_shader_variable from ir_variable class. + */ +static gl_shader_variable * +create_shader_variable(struct gl_shader_program *shProg, const ir_variable *in) +{ + gl_shader_variable *out = ralloc(shProg, struct gl_shader_variable); + if (!out) + return NULL; + + out->type = in->type; + out->name = ralloc_strdup(shProg, in->name);This can fail too, right? Might be nice to error-check.
Thanks, static analysis might complain about this, will fix.
+ + out->location = in->data.location; + out->index = in->data.index; + out->patch = in->data.patch; + out->mode = in->data.mode; + + return out; +} + static bool add_interface_variables(struct gl_shader_program *shProg, exec_list *ir, GLenum programInterface) @@ -3392,9 +3413,13 @@ add_interface_variables(struct gl_shader_program *shProg, if (strncmp(var->name, "gl_out_FragData", 15) == 0) continue; - if (!add_program_resource(shProg, programInterface, var, - build_stageref(shProg, var->name, - var->data.mode) | mask)) + gl_shader_variable *sha_v = create_shader_variable(shProg, var); + if (!sha_v) + return false; + + if (!add_program_resource(shProg, programInterface, sha_v, + build_stageref(shProg, sha_v->name, + sha_v->mode) | mask)) return false; } return true; @@ -3422,9 +3447,14 @@ add_packed_varyings(struct gl_shader_program *shProg, int stage) default: unreachable("unexpected type"); } - if (!add_program_resource(shProg, iface, var, - build_stageref(shProg, var->name, - var->data.mode))) + + gl_shader_variable *sha_v = create_shader_variable(shProg, var); + if (!sha_v) + return false; + + if (!add_program_resource(shProg, iface, sha_v, + build_stageref(shProg, sha_v->name, + sha_v->mode))) return false; } } @@ -3443,7 +3473,12 @@ add_fragdata_arrays(struct gl_shader_program *shProg) ir_variable *var = node->as_variable(); if (var) { assert(var->data.mode == ir_var_shader_out); - if (!add_program_resource(shProg, GL_PROGRAM_OUTPUT, var, + + gl_shader_variable *sha_v = create_shader_variable(shProg, var); + if (!sha_v) + return false; + + if (!add_program_resource(shProg, GL_PROGRAM_OUTPUT, sha_v, 1 << MESA_SHADER_FRAGMENT)) return false; } @@ -3723,8 +3758,13 @@ build_program_resource_list(struct gl_shader_program *shProg) if (shProg->SeparateShader) { if (!add_packed_varyings(shProg, input_stage)) return; - if (!add_packed_varyings(shProg, output_stage)) - return; + /* Only when dealing with multiple stages, otherwise we would have + * duplicate gl_shader_variable entries. + */ + if (input_stage != output_stage) { + if (!add_packed_varyings(shProg, output_stage)) + return; + } } if (!add_fragdata_arrays(shProg)) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index d6c1eb8..0316769 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2519,6 +2519,62 @@ struct gl_active_atomic_buffer }; /** + * Data container for shader queries. This holds only the minimal + * amount of required information for resource queries to work. + */ +struct gl_shader_variable +{ + /** + * Declared type of the variable + */ + const struct glsl_type *type; + + /** + * Declared name of the variable + */ + char *name; + + /** + * Storage location of the base of this variable + * + * The precise meaning of this field depends on the nature of the variable. + * + * - Vertex shader input: one of the values from \c gl_vert_attrib. + * - Vertex shader output: one of the values from \c gl_varying_slot. + * - Geometry shader input: one of the values from \c gl_varying_slot. + * - Geometry shader output: one of the values from \c gl_varying_slot. + * - Fragment shader input: one of the values from \c gl_varying_slot. + * - Fragment shader output: one of the values from \c gl_frag_result. + * - Uniforms: Per-stage uniform slot number for default uniform block. + * - Uniforms: Index within the uniform block definition for UBO members. + * - Non-UBO Uniforms: explicit location until linking then reused to + * store uniform slot number. + * - Other: This field is not currently used. + * + * If the variable is a uniform, shader input, or shader output, and the + * slot has not been assigned, the value will be -1. + */ + int location; + + /** + * Output index for dual source blending. + * + * \note + * The GLSL spec only allows the values 0 or 1 for the index in \b dual + * source blending. + */ + unsigned index:1; + unsigned patch:1;Perhaps say a few words about patch? This specifies whether a shader input/output is per-patch or not in TES/TCS. If you wanted to be stingy with bits, you could make index/patch be the same thing. But it's not worth it at this point.
OK, I'll add. This was just copy-paste from ir_variable_data which does not include any documentation for it.
+ + /** + * Storage class of the variable. + * + * \sa (n)ir_variable_mode + */ + unsigned mode:4; +}; + +/** * Active resource in a gl_shader_program */ struct gl_program_resource diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index b214691..683d835 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -56,7 +56,7 @@ const type * RESOURCE_ ## name (gl_program_resource *res) { \ return (type *) res->Data; \ } -DECL_RESOURCE_FUNC(VAR, ir_variable); +DECL_RESOURCE_FUNC(VAR, gl_shader_variable); DECL_RESOURCE_FUNC(UBO, gl_uniform_block); DECL_RESOURCE_FUNC(UNI, gl_uniform_storage); DECL_RESOURCE_FUNC(ATC, gl_active_atomic_buffer); @@ -101,14 +101,14 @@ _mesa_BindAttribLocation(GLhandleARB program, GLuint index, } static bool -is_active_attrib(const ir_variable *var) +is_active_attrib(const gl_shader_variable *var) { if (!var) return false; - switch (var->data.mode) { + switch (var->mode) { case ir_var_shader_in: - return var->data.location != -1; + return var->location != -1; case ir_var_system_value: /* From GL 4.3 core spec, section 11.1.1 (Vertex Attributes): @@ -116,9 +116,9 @@ is_active_attrib(const ir_variable *var) * are enumerated, including the special built-in inputs gl_VertexID * and gl_InstanceID." */ - return var->data.location == SYSTEM_VALUE_VERTEX_ID || - var->data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE || - var->data.location == SYSTEM_VALUE_INSTANCE_ID; + return var->location == SYSTEM_VALUE_VERTEX_ID || + var->location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE || + var->location == SYSTEM_VALUE_INSTANCE_ID; default: return false; @@ -163,7 +163,7 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index, return; } - const ir_variable *const var = RESOURCE_VAR(res); + const gl_shader_variable *const var = RESOURCE_VAR(res); if (!is_active_attrib(var)) return; @@ -174,8 +174,8 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index, * consider gl_VertexIDMESA as gl_VertexID for purposes of checking * active attributes. */ - if (var->data.mode == ir_var_system_value && - var->data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) { + if (var->mode == ir_var_system_value && + var->location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) { var_name = "gl_VertexID"; } @@ -427,7 +427,7 @@ _mesa_GetFragDataLocation(GLuint program, const GLchar *name) const char* _mesa_program_resource_name(struct gl_program_resource *res) { - const ir_variable *var; + const gl_shader_variable *var; switch (res->Type) { case GL_UNIFORM_BLOCK: case GL_SHADER_STORAGE_BLOCK: @@ -437,8 +437,8 @@ _mesa_program_resource_name(struct gl_program_resource *res) case GL_PROGRAM_INPUT: var = RESOURCE_VAR(res); /* Special case gl_VertexIDMESA -> gl_VertexID. */ - if (var->data.mode == ir_var_system_value && - var->data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) { + if (var->mode == ir_var_system_value && + var->location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) { return "gl_VertexID"; } /* fallthrough */ @@ -851,14 +851,14 @@ program_resource_location(struct gl_shader_program *shProg, && array_index >= RESOURCE_VAR(res)->type->length) { return -1; } - return RESOURCE_VAR(res)->data.location + array_index - VERT_ATTRIB_GENERIC0; + return RESOURCE_VAR(res)->location + array_index - VERT_ATTRIB_GENERIC0; case GL_PROGRAM_OUTPUT: /* If the output is an array, fail if the index is out of bounds. */ if (array_index > 0 && array_index >= RESOURCE_VAR(res)->type->length) { return -1; } - return RESOURCE_VAR(res)->data.location + array_index - FRAG_RESULT_DATA0; + return RESOURCE_VAR(res)->location + array_index - FRAG_RESULT_DATA0; case GL_UNIFORM: /* If the uniform is built-in, fail. */ if (RESOURCE_UNI(res)->builtin) @@ -938,7 +938,7 @@ _mesa_program_resource_location_index(struct gl_shader_program *shProg, if (!res || !(res->StageReferences & (1 << MESA_SHADER_FRAGMENT))) return -1; - return RESOURCE_VAR(res)->data.index; + return RESOURCE_VAR(res)->index; } static uint8_t @@ -1266,7 +1266,7 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg, case GL_LOCATION_INDEX: if (res->Type != GL_PROGRAM_OUTPUT) goto invalid_operation; - *val = RESOURCE_VAR(res)->data.index; + *val = RESOURCE_VAR(res)->index; return 1; case GL_NUM_COMPATIBLE_SUBROUTINES: @@ -1323,7 +1323,7 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg, switch (res->Type) { case GL_PROGRAM_INPUT: case GL_PROGRAM_OUTPUT: - *val = RESOURCE_VAR(res)->data.patch; + *val = RESOURCE_VAR(res)->patch; return 1; default: goto invalid_operation; -- 2.4.3 _______________________________________________ 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
