On 04/13/2015 12:15 PM, Martin Peres wrote:
On 01/04/15 15:14, Tapani Pälli wrote:Patch adds ProgramResourceList to gl_shader_program structure. List contains references to active program resources and is constructed during linking phase. This list will be used by follow-up patches to implement hooks for GL_ARB_program_interface_query. It can be also used to implement any of the older shader program query APIs. v2: code cleanups + note for SSBO and subroutines (Ilia Mirkin) Signed-off-by: Tapani Pälli <tapani.pa...@intel.com> --- src/glsl/linker.cpp | 179 ++++++++++++++++++++++++++++++++++++++++++++++ src/mesa/main/mtypes.h | 14 ++++ src/mesa/main/shaderobj.c | 6 ++ 3 files changed, 199 insertions(+) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 73432b2..a757425 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -2492,6 +2492,181 @@ check_explicit_uniform_locations(struct gl_context *ctx, delete uniform_map; } +static bool +add_program_resource(struct gl_shader_program *prog, GLenum type, + const void *data, uint8_t stages) +{ + assert(data); + + /* If resource already exists, do not add it again. */ + for (unsigned i = 0; i < prog->NumProgramResourceList; i++) + if (prog->ProgramResourceList[i].Data == data) + return true; + + prog->ProgramResourceList = + reralloc(prog, + prog->ProgramResourceList, + gl_program_resource, + prog->NumProgramResourceList + 1); + + if (!prog->ProgramResourceList) { + linker_error(prog, "Out of memory during linking.\n"); + return false; + } + + struct gl_program_resource *res = + &prog->ProgramResourceList[prog->NumProgramResourceList]; + + res->Type = type; + res->Data = data; + res->StageReferences = stages; + + prog->NumProgramResourceList++; + + return true; +} + +/** + * Function builds a stage reference bitmask from variable name. + */ +static uint8_tCould this become a uint16_t? With both tessellation, compute and geometry, we are getting close to a 8. If it is a little tricky, then adding an assert somewhere to make sure that MESA_SHADER_STAGES < 8 would be great (along with a comment saying what needs to be changed).
Sure, I'll add the assertion to be safe. Then if there will ever be new shader stages the type can be revisited.
+build_stageref(struct gl_shader_program *shProg, const char *name) +{ + uint8_t stages = 0; + for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { + struct gl_shader *sh = shProg->_LinkedShaders[i]; + if (!sh) + continue; + ir_variable *var = sh->symbols->get_variable(name); + if (var) + stages |= (1 << i); + } + return stages; +} + +/** + * Builds up a list of program resources that point to existing + * resource data. + */ +static void +build_program_resource_list(struct gl_context *ctx, + struct gl_shader_program *shProg) +{ + /* Rebuild resource list. */ + if (shProg->ProgramResourceList) { + ralloc_free(shProg->ProgramResourceList); + shProg->ProgramResourceList = NULL; + shProg->NumProgramResourceList = 0; + } + + int input_stage = MESA_SHADER_STAGES, output_stage = 0; + + /* Determine first input and final output stage. These are used to + * detect which variables should be enumerated in the resource list + * for GL_PROGRAM_INPUT and GL_PROGRAM_OUTPUT. + */ + for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { + struct gl_shader *sh = shProg->_LinkedShaders[i];No need for this intermediate variable.
will remove
+ if (!sh) + continue; + if (input_stage == MESA_SHADER_STAGES) + input_stage = i; + output_stage = i; + } + + for (int i = 0; i < MESA_SHADER_STAGES; i++) { + struct gl_shader *sh = shProg->_LinkedShaders[i]; + + if (!sh || (i != input_stage && i != output_stage))This looks like an ugly way for not creating a function called once of input_stage and once on output_stage. Given the length of the function, this would not be a bad idea to move the following hunk to a separate function anyway.
OK, I'll refactor this part.
+ continue; + + /* Add inputs and outputs to the resource list. */ + foreach_in_list(ir_instruction, node, sh->ir) { + ir_variable *var = node->as_variable(); + GLenum iface; + + if (!var) + continue; + + switch (var->data.mode) { + /* From GL 4.3 core spec, section 11.1.1 (Vertex Attributes): + * "For GetActiveAttrib, all active vertex shader input variables + * are enumerated, including the special built-in inputs gl_VertexID + * and gl_InstanceID." + */ + case ir_var_system_value: + if (var->data.location != SYSTEM_VALUE_VERTEX_ID && + var->data.location != SYSTEM_VALUE_VERTEX_ID_ZERO_BASE && + var->data.location != SYSTEM_VALUE_INSTANCE_ID) + continue; + case ir_var_shader_in: + if (i != input_stage) + continue; + iface = GL_PROGRAM_INPUT; + break; + case ir_var_shader_out: + if (i != output_stage) + continue; + iface = GL_PROGRAM_OUTPUT; + break; + default: + continue; + }; + + if (!add_program_resource(shProg, iface, var, + build_stageref(shProg, var->name))) + return; + } + } + + /* Add transform feedback varyings. */ + if (shProg->LinkedTransformFeedback.NumVarying > 0) { + for (int i = 0; i < shProg->LinkedTransformFeedback.NumVarying; i++) { + uint8_t stageref = + build_stageref(shProg, + shProg->LinkedTransformFeedback.Varyings[i].Name);Shouldn't stageref only have the bit corresponding to the latest geometry-related stage set, since TF only happens there?
It's a varying so I would expect it to be referenced by multiple stages.
+ if (!add_program_resource(shProg, GL_TRANSFORM_FEEDBACK_VARYING, + &shProg->LinkedTransformFeedback.Varyings[i], + stageref)) + return; + } + } + + /* Add uniforms from uniform storage. */ + for (unsigned i = 0; i < shProg->NumUserUniformStorage; i++) { + /* Do not add uniforms internally used by Mesa. */ + if (shProg->UniformStorage[i].hidden) + continue; + + uint8_t stageref = + build_stageref(shProg, shProg->UniformStorage[i].name); + if (!add_program_resource(shProg, GL_UNIFORM, + &shProg->UniformStorage[i], stageref)) + return; + } + + /* Add program uniform blocks. */ + for (unsigned i = 0; i < shProg->NumUniformBlocks; i++) { + if (!add_program_resource(shProg, GL_UNIFORM_BLOCK, + &shProg->UniformBlocks[i], 0)) + return; + } + + /* Add atomic counter buffers. */ + for (unsigned i = 0; i < shProg->NumAtomicBuffers; i++) { + if (!add_program_resource(shProg, GL_ATOMIC_COUNTER_BUFFER, + &shProg->AtomicBuffers[i], 0)) + return; + } + + /* TODO - following extensions will require more resource types: + * + * GL_ARB_shader_storage_buffer_object + * GL_ARB_shader_subroutine + */Very good :) This could also be mentioned in GL3.txt.
Yeah, can be added.
+} + + void link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) { @@ -2900,6 +3075,10 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) } } + build_program_resource_list(ctx, prog); + if (!prog->LinkStatus)This is legal to return a resource list when the program did not link but it is not necessary. Your call :)
Right, It's only used here as a method to detect OOM condition, ralloc calls inside may fail.
+ goto done; + /* FINISHME: Assign fragment shader output locations. */ done: diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 8e1dba6..e0dd099 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2762,6 +2762,16 @@ struct gl_active_atomic_buffer }; /** + * Active resource in a gl_shader_program + */ +struct gl_program_resource +{ + GLenum Type; /** Program interface type. */ + const void *Data; /** Pointer to resource associated data structure. */ + uint8_t StageReferences; /** Bitmask of shader stage references. */ +}; + +/** * A GLSL program object. * Basically a linked collection of vertex and fragment shaders. */ @@ -2935,6 +2945,10 @@ struct gl_shader_program */ struct gl_shader *_LinkedShaders[MESA_SHADER_STAGES]; + /** List of all active resources after linking. */ + struct gl_program_resource *ProgramResourceList; + unsigned NumProgramResourceList; + /* True if any of the fragment shaders attached to this program use: * #extension ARB_fragment_coord_conventions: enable */ diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c index d7620c8..e428960 100644 --- a/src/mesa/main/shaderobj.c +++ b/src/mesa/main/shaderobj.c @@ -315,6 +315,12 @@ _mesa_clear_shader_program_data(struct gl_shader_program *shProg) ralloc_free(shProg->AtomicBuffers); shProg->AtomicBuffers = NULL; shProg->NumAtomicBuffers = 0; + + if (shProg->ProgramResourceList) { + ralloc_free(shProg->ProgramResourceList); + shProg->ProgramResourceList = NULL; + shProg->NumProgramResourceList = 0; + } }
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
