[Mesa-dev] [PATCH] mesa: use gl_shader_variable in program resource list
Tapani Pälli
tapani.palli at intel.com
Mon Nov 2 01:12:14 PST 2015
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.
>> +
>> + /**
>> + * Storage class of the variable.
>> + *
>> + * \sa (n)ir_variable_mode
>> + */
>> + unsigned mode:4;
>> +};
>> +
>> +/**
>> * Active resource in a gl_shader_program
>> */
>> struct gl_program_resource
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index b214691..683d835 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -56,7 +56,7 @@ const type * RESOURCE_ ## name (gl_program_resource *res) { \
>> return (type *) res->Data; \
>> }
>>
>> -DECL_RESOURCE_FUNC(VAR, ir_variable);
>> +DECL_RESOURCE_FUNC(VAR, gl_shader_variable);
>> DECL_RESOURCE_FUNC(UBO, gl_uniform_block);
>> DECL_RESOURCE_FUNC(UNI, gl_uniform_storage);
>> DECL_RESOURCE_FUNC(ATC, gl_active_atomic_buffer);
>> @@ -101,14 +101,14 @@ _mesa_BindAttribLocation(GLhandleARB program, GLuint index,
>> }
>>
>> static bool
>> -is_active_attrib(const ir_variable *var)
>> +is_active_attrib(const gl_shader_variable *var)
>> {
>> if (!var)
>> return false;
>>
>> - switch (var->data.mode) {
>> + switch (var->mode) {
>> case ir_var_shader_in:
>> - return var->data.location != -1;
>> + return var->location != -1;
>>
>> case ir_var_system_value:
>> /* From GL 4.3 core spec, section 11.1.1 (Vertex Attributes):
>> @@ -116,9 +116,9 @@ is_active_attrib(const ir_variable *var)
>> * are enumerated, including the special built-in inputs gl_VertexID
>> * and gl_InstanceID."
>> */
>> - return var->data.location == SYSTEM_VALUE_VERTEX_ID ||
>> - var->data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE ||
>> - var->data.location == SYSTEM_VALUE_INSTANCE_ID;
>> + return var->location == SYSTEM_VALUE_VERTEX_ID ||
>> + var->location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE ||
>> + var->location == SYSTEM_VALUE_INSTANCE_ID;
>>
>> default:
>> return false;
>> @@ -163,7 +163,7 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index,
>> return;
>> }
>>
>> - const ir_variable *const var = RESOURCE_VAR(res);
>> + const gl_shader_variable *const var = RESOURCE_VAR(res);
>>
>> if (!is_active_attrib(var))
>> return;
>> @@ -174,8 +174,8 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index,
>> * consider gl_VertexIDMESA as gl_VertexID for purposes of checking
>> * active attributes.
>> */
>> - if (var->data.mode == ir_var_system_value &&
>> - var->data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) {
>> + if (var->mode == ir_var_system_value &&
>> + var->location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) {
>> var_name = "gl_VertexID";
>> }
>>
>> @@ -427,7 +427,7 @@ _mesa_GetFragDataLocation(GLuint program, const GLchar *name)
>> const char*
>> _mesa_program_resource_name(struct gl_program_resource *res)
>> {
>> - const ir_variable *var;
>> + const gl_shader_variable *var;
>> switch (res->Type) {
>> case GL_UNIFORM_BLOCK:
>> case GL_SHADER_STORAGE_BLOCK:
>> @@ -437,8 +437,8 @@ _mesa_program_resource_name(struct gl_program_resource *res)
>> case GL_PROGRAM_INPUT:
>> var = RESOURCE_VAR(res);
>> /* Special case gl_VertexIDMESA -> gl_VertexID. */
>> - if (var->data.mode == ir_var_system_value &&
>> - var->data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) {
>> + if (var->mode == ir_var_system_value &&
>> + var->location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) {
>> return "gl_VertexID";
>> }
>> /* fallthrough */
>> @@ -851,14 +851,14 @@ program_resource_location(struct gl_shader_program *shProg,
>> && array_index >= RESOURCE_VAR(res)->type->length) {
>> return -1;
>> }
>> - return RESOURCE_VAR(res)->data.location + array_index - VERT_ATTRIB_GENERIC0;
>> + return RESOURCE_VAR(res)->location + array_index - VERT_ATTRIB_GENERIC0;
>> case GL_PROGRAM_OUTPUT:
>> /* If the output is an array, fail if the index is out of bounds. */
>> if (array_index > 0
>> && array_index >= RESOURCE_VAR(res)->type->length) {
>> return -1;
>> }
>> - return RESOURCE_VAR(res)->data.location + array_index - FRAG_RESULT_DATA0;
>> + return RESOURCE_VAR(res)->location + array_index - FRAG_RESULT_DATA0;
>> case GL_UNIFORM:
>> /* If the uniform is built-in, fail. */
>> if (RESOURCE_UNI(res)->builtin)
>> @@ -938,7 +938,7 @@ _mesa_program_resource_location_index(struct gl_shader_program *shProg,
>> if (!res || !(res->StageReferences & (1 << MESA_SHADER_FRAGMENT)))
>> return -1;
>>
>> - return RESOURCE_VAR(res)->data.index;
>> + return RESOURCE_VAR(res)->index;
>> }
>>
>> static uint8_t
>> @@ -1266,7 +1266,7 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg,
>> case GL_LOCATION_INDEX:
>> if (res->Type != GL_PROGRAM_OUTPUT)
>> goto invalid_operation;
>> - *val = RESOURCE_VAR(res)->data.index;
>> + *val = RESOURCE_VAR(res)->index;
>> return 1;
>>
>> case GL_NUM_COMPATIBLE_SUBROUTINES:
>> @@ -1323,7 +1323,7 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg,
>> switch (res->Type) {
>> case GL_PROGRAM_INPUT:
>> case GL_PROGRAM_OUTPUT:
>> - *val = RESOURCE_VAR(res)->data.patch;
>> + *val = RESOURCE_VAR(res)->patch;
>> return 1;
>> default:
>> goto invalid_operation;
>> --
>> 2.4.3
>>
>> _______________________________________________
>> 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