[Mesa-dev] [PATCH 02/23] mesa/glsl: build list of program resources during linking

Tapani Pälli tapani.palli at intel.com
Mon Mar 16 05:22:06 PDT 2015



On 03/13/2015 06:16 PM, Ilia Mirkin wrote:
> On Fri, Mar 13, 2015 at 4:37 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.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/glsl/linker.cpp       | 175 ++++++++++++++++++++++++++++++++++++++++++++++
>>   src/mesa/main/mtypes.h    |  14 ++++
>>   src/mesa/main/shaderobj.c |   6 ++
>>   3 files changed, 195 insertions(+)
>>
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index 0c44677..2396848 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -2503,6 +2503,177 @@ 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++) {
>> +      struct gl_program_resource *res =
>> +         &prog->ProgramResourceList[i];
>
> This should fit on the previous line...

yep, it is also a bit unnecessary .. will remove and just use table 
directly, I think this used to do a bit more checking but checking just 
for the Data is enough

>> +      if (res->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);
>
> and this...

yep, will fix

>> +      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)
>> +         continue;
>
> Would you ever want to do this for stages that are not the input or
> output stage? I think it'd be nice to avoid the iteration in those
> cases...

That is true and will be faster, I'll change this.

>> +
>> +      /* 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 ||
>> +             (var->data.mode == ir_var_shader_in && i != input_stage) ||
>> +             (var->data.mode == ir_var_shader_out && i != output_stage))
>> +            continue;
>
> IMO it'd be more readable to integrate these into the switch below. i.e.
>
> if (!var)
>    continue
>
> and then
>
> switch (...)
>
> case in:
>    if (i != input_stage)
>       continue;
>    iface = input
>    break;

Heh, just like I used to have before for some reason I moved them there 
on top. I agree, I'll put them back like this.

>> +
>> +         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:
>> +            iface = GL_PROGRAM_INPUT;
>> +            break;
>> +         case ir_var_shader_out:
>> +            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);
>> +         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++) {
>
> i = 0
>
>> +      /* 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;
>> +   }
>
> And I guess SSBO blocks, but those aren't a thing in mesa yet...
> Perhaps add a TODO?

OK, will add, SSBO and subroutines will have to be added in the future. 
Thanks!

>> +}
>> +
>> +
>>   void
>>   link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>>   {
>> @@ -2904,6 +3075,10 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>>         }
>>      }
>>
>> +   build_program_resource_list(ctx, prog);
>> +   if (!prog->LinkStatus)
>> +      goto done;
>> +
>>      /* FINISHME: Assign fragment shader output locations. */
>>
>>   done:
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index c43c6ac..4f18044 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -2749,6 +2749,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.
>>    */
>> @@ -2922,6 +2932,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;
>> +   }
>>   }
>>
>>
>> --
>> 2.1.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list