[Mesa-dev] [PATCH] mesa: use gl_shader_variable in program resource list

Marek Olšák maraeo at gmail.com
Wed Dec 30 11:53:41 PST 2015


On Mon, Nov 2, 2015 at 10:12 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>
>
> On 11/02/2015 09:16 AM, Ilia Mirkin wrote:
>>
>> On Mon, Nov 2, 2015 at 1:58 AM, Tapani Pälli <tapani.palli at intel.com>
>> wrote:
>>>
>>> Patch changes linker to allocate gl_shader_variable instead of using
>>> ir_variable. This makes it possible to get rid of ir_variables and ir
>>> in memory after linking.
>>>
>>> v2: check that we do not create duplicate entries with
>>>      packed varyings
>>>
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> ---
>>>   src/glsl/linker.cpp            | 58
>>> +++++++++++++++++++++++++++++++++++-------
>>>   src/mesa/main/mtypes.h         | 56
>>> ++++++++++++++++++++++++++++++++++++++++
>>>   src/mesa/main/shader_query.cpp | 36 +++++++++++++-------------
>>>   3 files changed, 123 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>>> index 48dd2d3..d0353b4 100644
>>> --- a/src/glsl/linker.cpp
>>> +++ b/src/glsl/linker.cpp
>>> @@ -3341,6 +3341,27 @@ build_stageref(struct gl_shader_program *shProg,
>>> const char *name,
>>>      return stages;
>>>   }
>>>
>>> +/**
>>> + * Create gl_shader_variable from ir_variable class.
>>> + */
>>> +static gl_shader_variable *
>>> +create_shader_variable(struct gl_shader_program *shProg, const
>>> ir_variable *in)
>>> +{
>>> +   gl_shader_variable *out = ralloc(shProg, struct gl_shader_variable);
>>> +   if (!out)
>>> +      return NULL;
>>> +
>>> +   out->type = in->type;
>>> +   out->name = ralloc_strdup(shProg, in->name);
>>
>>
>> This can fail too, right? Might be nice to error-check.
>
>
> Thanks, static analysis might complain about this, will fix.
>
>
>>> +
>>> +   out->location = in->data.location;
>>> +   out->index = in->data.index;
>>> +   out->patch = in->data.patch;
>>> +   out->mode = in->data.mode;
>>> +
>>> +   return out;
>>> +}
>>> +
>>>   static bool
>>>   add_interface_variables(struct gl_shader_program *shProg,
>>>                           exec_list *ir, GLenum programInterface)
>>> @@ -3392,9 +3413,13 @@ add_interface_variables(struct gl_shader_program
>>> *shProg,
>>>         if (strncmp(var->name, "gl_out_FragData", 15) == 0)
>>>            continue;
>>>
>>> -      if (!add_program_resource(shProg, programInterface, var,
>>> -                                build_stageref(shProg, var->name,
>>> -                                               var->data.mode) | mask))
>>> +      gl_shader_variable *sha_v = create_shader_variable(shProg, var);
>>> +      if (!sha_v)
>>> +         return false;
>>> +
>>> +      if (!add_program_resource(shProg, programInterface, sha_v,
>>> +                                build_stageref(shProg, sha_v->name,
>>> +                                               sha_v->mode) | mask))
>>>            return false;
>>>      }
>>>      return true;
>>> @@ -3422,9 +3447,14 @@ add_packed_varyings(struct gl_shader_program
>>> *shProg, int stage)
>>>            default:
>>>               unreachable("unexpected type");
>>>            }
>>> -         if (!add_program_resource(shProg, iface, var,
>>> -                                   build_stageref(shProg, var->name,
>>> -                                                  var->data.mode)))
>>> +
>>> +         gl_shader_variable *sha_v = create_shader_variable(shProg,
>>> var);
>>> +         if (!sha_v)
>>> +            return false;
>>> +
>>> +         if (!add_program_resource(shProg, iface, sha_v,
>>> +                                   build_stageref(shProg, sha_v->name,
>>> +                                                  sha_v->mode)))
>>>               return false;
>>>         }
>>>      }
>>> @@ -3443,7 +3473,12 @@ add_fragdata_arrays(struct gl_shader_program
>>> *shProg)
>>>         ir_variable *var = node->as_variable();
>>>         if (var) {
>>>            assert(var->data.mode == ir_var_shader_out);
>>> -         if (!add_program_resource(shProg, GL_PROGRAM_OUTPUT, var,
>>> +
>>> +         gl_shader_variable *sha_v = create_shader_variable(shProg,
>>> var);
>>> +         if (!sha_v)
>>> +            return false;
>>> +
>>> +         if (!add_program_resource(shProg, GL_PROGRAM_OUTPUT, sha_v,
>>>                                      1 << MESA_SHADER_FRAGMENT))
>>>               return false;
>>>         }
>>> @@ -3723,8 +3758,13 @@ build_program_resource_list(struct
>>> gl_shader_program *shProg)
>>>      if (shProg->SeparateShader) {
>>>         if (!add_packed_varyings(shProg, input_stage))
>>>            return;
>>> -      if (!add_packed_varyings(shProg, output_stage))
>>> -         return;
>>> +      /* Only when dealing with multiple stages, otherwise we would have
>>> +       * duplicate gl_shader_variable entries.
>>> +       */
>>> +      if (input_stage != output_stage) {
>>> +         if (!add_packed_varyings(shProg, output_stage))
>>> +            return;
>>> +      }
>>>      }
>>>
>>>      if (!add_fragdata_arrays(shProg))
>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>>> index d6c1eb8..0316769 100644
>>> --- a/src/mesa/main/mtypes.h
>>> +++ b/src/mesa/main/mtypes.h
>>> @@ -2519,6 +2519,62 @@ struct gl_active_atomic_buffer
>>>   };
>>>
>>>   /**
>>> + * Data container for shader queries. This holds only the minimal
>>> + * amount of required information for resource queries to work.
>>> + */
>>> +struct gl_shader_variable
>>> +{
>>> +   /**
>>> +    * Declared type of the variable
>>> +    */
>>> +   const struct glsl_type *type;
>>> +
>>> +   /**
>>> +    * Declared name of the variable
>>> +    */
>>> +   char *name;
>>> +
>>> +   /**
>>> +    * Storage location of the base of this variable
>>> +    *
>>> +    * The precise meaning of this field depends on the nature of the
>>> variable.
>>> +    *
>>> +    *   - Vertex shader input: one of the values from \c gl_vert_attrib.
>>> +    *   - Vertex shader output: one of the values from \c
>>> gl_varying_slot.
>>> +    *   - Geometry shader input: one of the values from \c
>>> gl_varying_slot.
>>> +    *   - Geometry shader output: one of the values from \c
>>> gl_varying_slot.
>>> +    *   - Fragment shader input: one of the values from \c
>>> gl_varying_slot.
>>> +    *   - Fragment shader output: one of the values from \c
>>> gl_frag_result.
>>> +    *   - Uniforms: Per-stage uniform slot number for default uniform
>>> block.
>>> +    *   - Uniforms: Index within the uniform block definition for UBO
>>> members.
>>> +    *   - Non-UBO Uniforms: explicit location until linking then reused
>>> to
>>> +    *     store uniform slot number.
>>> +    *   - Other: This field is not currently used.
>>> +    *
>>> +    * If the variable is a uniform, shader input, or shader output, and
>>> the
>>> +    * slot has not been assigned, the value will be -1.
>>> +    */
>>> +   int location;
>>> +
>>> +   /**
>>> +    * Output index for dual source blending.
>>> +    *
>>> +    * \note
>>> +    * The GLSL spec only allows the values 0 or 1 for the index in \b
>>> dual
>>> +    * source blending.
>>> +    */
>>> +   unsigned index:1;
>>> +   unsigned patch:1;
>>
>>
>> Perhaps say a few words about patch? This specifies whether a shader
>> input/output is per-patch or not in TES/TCS. If you wanted to be
>> stingy with bits, you could make index/patch be the same thing. But
>> it's not worth it at this point.
>
>
> OK, I'll add. This was just copy-paste from ir_variable_data which does not
> include any documentation for it.

Hi,

What's the state of this?

Thanks,
Marek


More information about the mesa-dev mailing list