[Mesa-dev] [PATCH v2 03/24] mesa/glsl: build list of program resources during linking
Tapani Pälli
tapani.palli at intel.com
Mon Apr 13 02:30:34 PDT 2015
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.palli at 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_t
> Could 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;
>> + }
>> }
>
More information about the mesa-dev
mailing list