[Mesa-dev] [PATCH] mesa: use gl_shader_variable in program resource list
Tapani Pälli
tapani.palli at intel.com
Sun Jan 3 22:21:30 PST 2016
On 12/30/2015 09:53 PM, Marek Olšák wrote:
> 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?
I have v3 available here with fixes to Ilia's comments:
http://cgit.freedesktop.org/~tpalli/mesa/commit/?h=floating
I'll rebase and send it to the list.
> Thanks,
> Marek
>
More information about the mesa-dev
mailing list