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

Martin Peres martin.peres at linux.intel.com
Mon Apr 13 02:15:40 PDT 2015



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).
> +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.
> +      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.
> +         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?
> +         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.
> +}
> +
> +
>   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 :)
> +      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