[Mesa-dev] [PATCH v2 03/24] mesa/glsl: build list of program resources during linking

[Tapani]

On 04/21/2015 04:49 AM, Matt Turner wrote:
> On Wed, Apr 1, 2015 at 5:14 AM, Tapani Pälli <tapani.palli at intel.com> 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
>> +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];
>> +      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))
>> +         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;
> Coverity is warning about this... and rightly so.
>
> Either the indentation on the "continue" statement is wrong and it's
> missing a /* fallthrough */ comment, or something more serious is
> wrong.

It is missing a /* fallthrough */ comment. I'll send a patch that adds 
comments for both of the fallthrough cases you found.

// Tapani