[Mesa-dev] [PATCH 2/3] glsl: enforce fragment shader input restrictions in GLSL ES 3.10
---
src/glsl/ast_to_hir.cpp | 45 +
1 file changed, 45 insertions(+)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index d7ecc35..8236ff9 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3610,6 +3610,51 @@ ast_declarator_list::hir(exec_list *instructions,
}
handle_geometry_shader_input_decl(state, loc, var);
+ } else if (state->stage == MESA_SHADER_FRAGMENT) {
+/* From section 4.3.4 (Input Variables) of the GLSL ES 3.10 spec:
+ *
+ * It is a compile-time error to declare a fragment shader
+ * input with, or that contains, any of the following types:
+ *
+ * * A boolean type
+ * * An opaque type
+ * * An array of arrays
+ * * An array of structures
+ * * A structure containing an array
+ * * A structure containing a structure
+ */
+if (state->es_shader) {
+ const glsl_type *check_type = var->type->without_array();
+ if (check_type->is_boolean() ||
+ check_type->contains_opaque()) {
+ _mesa_glsl_error(&loc, state,
+ "fragment shader input cannot have type %s",
+ check_type->name);
+ }
+ if (var->type->is_array() &&
+ var->type->fields.array->is_array()) {
+ _mesa_glsl_error(&loc, state,
+ "%s shader output "
+ "cannot have an array of arrays",
+ _mesa_shader_stage_to_string(state->stage));
+ }
+ if (var->type->is_array() &&
+ var->type->fields.array->is_record()) {
+ _mesa_glsl_error(&loc, state,
+ "fragment shader input "
+ "cannot have an array of structs");
+ }
+ if (var->type->is_record()) {
+ for (unsigned i = 0; i < var->type->length; i++) {
+ if (var->type->fields.structure[i].type->is_array() ||
+ var->type->fields.structure[i].type->is_record())
+_mesa_glsl_error(&loc, state,
+ "fragement shader input cannot have "
+ "a struct that contains an "
+ "array or struct");
+ }
+ }
+}
}
} else if (var->data.mode == ir_var_shader_out) {
const glsl_type *check_type = var->type->without_array();
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] glsl: enforce restriction on AoA interface blocks in GLSL ES 3.10
---
src/glsl/ast_to_hir.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 8236ff9..93e0a82 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -5851,6 +5851,17 @@ ast_interface_block::hir(exec_list *instructions,
const glsl_type *block_array_type =
process_array_type(&loc, block_type, this->array_specifier, state);
+ /* From section 4.3.9 (Interface Blocks) of the GLSL ES 3.10 spec:
+ *
+ * * Arrays of arrays of blocks are not allowed
+ */
+ if (state->es_shader && block_array_type->is_array() &&
+ block_array_type->fields.array->is_array()) {
+_mesa_glsl_error(&loc, state,
+ "arrays of arrays interface blocks are"
+ " not allowed");
+ }
+
if (this->layout.flags.q.explicit_binding)
validate_binding_qualifier(state, &loc, block_array_type,
&this->layout);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] glsl: enforce output variable rules for GLSL ES 3.10
Some rules are already applied this just adds the missing ones.
---
src/glsl/ast_to_hir.cpp | 49 +
1 file changed, 49 insertions(+)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 578711a..d7ecc35 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3656,6 +3656,55 @@ ast_declarator_list::hir(exec_list *instructions,
"type %s", check_type->name);
}
}
+
+ /* From section 4.3.6 (Output Variables) of the GLSL ES 3.10 spec:
+ *
+ * It is a compile-time error to declare a vertex shader output
+ * with, or that contains, any of the following types:
+ *
+ * * A boolean type
+ * * An opaque type
+ * * An array of arrays
+ * * An array of structures
+ * * A structure containing an array
+ * * A structure containing a structure
+ *
+ * It is a compile-time error to declare a fragment shader output
+ * with, or that contains, any of the following types:
+ *
+ * * A boolean type
+ * * An opaque type
+ * * A matrix
+ * * A structure
+ * * An array of array
+ */
+ if (state->es_shader) {
+if (var->type->is_array() &&
+var->type->fields.array->is_array()) {
+ _mesa_glsl_error(&loc, state,
+"%s shader output "
+"cannot have an array of arrays",
+_mesa_shader_stage_to_string(state->stage));
+}
+if (state->stage == MESA_SHADER_VERTEX) {
+ if (var->type->is_array() &&
+ var->type->fields.array->is_record()) {
+ _mesa_glsl_error(&loc, state,
+ "vertex shader output "
+ "cannot have an array of structs");
+ }
+ if (var->type->is_record()) {
+ for (unsigned i = 0; i < var->type->length; i++) {
+ if (var->type->fields.structure[i].type->is_array() ||
+ var->type->fields.structure[i].type->is_record())
+_mesa_glsl_error(&loc, state,
+ "vertex shader output cannot have a "
+ "struct that contains an "
+ "array or struct");
+ }
+ }
+}
+ }
}
/* Integer fragment inputs must be qualified with 'flat'. In GLSL ES,
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] glsl: enforce restriction on AoA interface blocks in GLSL ES 3.10
Just noticed this applys on top of a previous patch I've sent.
http://lists.freedesktop.org/archives/mesa-dev/2015-June/085713.html
On Fri, 2015-06-12 at 16:52 +1000, Timothy Arceri wrote:
> ---
> src/glsl/ast_to_hir.cpp | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 8236ff9..93e0a82 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -5851,6 +5851,17 @@ ast_interface_block::hir(exec_list *instructions,
> const glsl_type *block_array_type =
> process_array_type(&loc, block_type, this->array_specifier,
> state);
>
> + /* From section 4.3.9 (Interface Blocks) of the GLSL ES 3.10 spec:
> + *
> + * * Arrays of arrays of blocks are not allowed
> + */
> + if (state->es_shader && block_array_type->is_array() &&
> + block_array_type->fields.array->is_array()) {
> +_mesa_glsl_error(&loc, state,
> + "arrays of arrays interface blocks are"
> + " not allowed");
> + }
> +
> if (this->layout.flags.q.explicit_binding)
> validate_binding_qualifier(state, &loc, block_array_type,
> &this->layout);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: remove cross validation of interpolation qualifier with GLSL 4.40
On Tue, 9 Jun 2015 11:06:56 +0300
Tapani Pälli wrote:
> Signed-off-by: Tapani Pälli
> ---
> src/glsl/link_varyings.cpp | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 7b2d4bd..a19ee39 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -128,7 +128,17 @@ cross_validate_types_and_qualifiers(struct
> gl_shader_program *prog, return;
> }
>
> - if (input->data.interpolation != output->data.interpolation) {
> + /* GLSL >= 4.40 removes text requiring interpolation qualifiers
> +* to match cross stage, they must only match within the same
> stage.
> +*
> +* From page 84 (page 90 of the PDF) of the GLSL 4.40 spec:
> +*
> +* "It is a link-time error if, within the same stage, the
> interpolation
> +* qualifiers of variables of the same name do not match.
> +*
> +*/
> + if (input->data.interpolation != output->data.interpolation &&
> + prog->Version < 440) {
>linker_error(prog,
> "%s shader output `%s' specifies %s "
> "interpolation qualifier, "
Reviewed-by: Timothy Arceri
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: fix compile error message
---
src/glsl/ast_to_hir.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 350f6ed..578711a 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3644,7 +3644,7 @@ ast_declarator_list::hir(exec_list *instructions,
if (check_type->is_record() || check_type->is_matrix())
_mesa_glsl_error(&loc, state,
"fragment shader output "
-"cannot have struct or array type");
+"cannot have struct or matrix type");
switch (check_type->base_type) {
case GLSL_TYPE_UINT:
case GLSL_TYPE_INT:
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] Revert "glsl: remove restriction on unsized arrays in GLSL ES 3.10"
This reverts commit adee54f8269c5e9f4fde91d19f0e465afc8f14d8.
Further down in the GLSL ES 3.10 spec it say:
"If an array is declared as the last member of a shader storage block
and the size is not specified at compile-time, it is sized at run-time.
In all other cases, arrays are sized only at compile-time."
---
Not sure if I needed to sent this to the list since I wrote the
initial patch.
src/glsl/ast_to_hir.cpp | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index fc24305..259e01e 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3943,15 +3943,7 @@ ast_declarator_list::hir(exec_list *instructions,
decl->identifier);
}
- /* GLSL ES 3.10 removes the restriction on unsized arrays.
- *
- * Section 4.1.9 (Arrays) of the GLSL ES 3.10 spec says:
- *
- *"Variables of the same type can be aggregated into arrays by
- * declaring a name followed by brackets ([ ]) enclosing an
- * optional size."
- */
- if (state->es_shader && state->language_version < 310) {
+ if (state->es_shader) {
const glsl_type *const t = (earlier == NULL)
? var->type : earlier->type;
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 36/82] glsl: add support for unsized arrays in shader storage blocks
On Wed, 2015-06-03 at 09:01 +0200, Iago Toral Quiroga wrote:
> From: Samuel Iglesias Gonsalvez
>
> They only can be defined in the last position of the shader
> storage blocks.
>
> When an unsized array is used in different shaders, it might be
> converted in different sized arrays, avoid get a linker error
> in that case.
>
> Signed-off-by: Samuel Iglesias Gonsalvez
> ---
> src/glsl/ast_array_index.cpp | 6 +--
> src/glsl/ast_to_hir.cpp | 32 +
> src/glsl/ir.cpp | 1 +
> src/glsl/ir.h| 14 ++
> src/glsl/linker.cpp | 107
> ---
> 5 files changed, 121 insertions(+), 39 deletions(-)
>
> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
> index 752d86f..481efae 100644
> --- a/src/glsl/ast_array_index.cpp
> +++ b/src/glsl/ast_array_index.cpp
> @@ -106,7 +106,6 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE
> *loc,
> }
> }
>
> -
> ir_rvalue *
> _mesa_ast_array_index_to_hir(void *mem_ctx,
>struct _mesa_glsl_parse_state *state,
> @@ -182,8 +181,9 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
>if (array->type->is_array())
> update_max_array_access(array, idx, &loc, state);
> } else if (const_index == NULL && array->type->is_array()) {
> - if (array->type->is_unsized_array()) {
> - _mesa_glsl_error(&loc, state, "unsized array index must be constant");
> + if (array->type->is_unsized_array() &&
> + array->variable_referenced()->data.mode != ir_var_shader_storage) {
> +_mesa_glsl_error(&loc, state, "unsized array index must be
> constant");
>} else if (array->type->fields.array->is_interface()
> && array->variable_referenced()->data.mode == ir_var_uniform
> && !state->is_version(400, 0) &&
> !state->ARB_gpu_shader5_enable) {
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index e1e5ca9..3ec28dd 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -5507,6 +5507,19 @@ private:
> bool found;
> };
>
> +static bool
> +is_unsized_array_last_element(ir_variable *v)
> +{
> + const glsl_type *interface_type = v->get_interface_type();
> + int length = interface_type->length;
> +
> + assert(v->type->is_unsized_array());
> +
> + /* Check if it is the last element of the interface */
> + if (strcmp(interface_type->fields.structure[length-1].name, v->name) == 0)
> + return true;
> + return false;
> +}
>
> ir_rvalue *
> ast_interface_block::hir(exec_list *instructions,
> @@ -5828,6 +5841,15 @@ ast_interface_block::hir(exec_list *instructions,
> var->data.explicit_binding = this->layout.flags.q.explicit_binding;
> var->data.binding = this->layout.binding;
>
> + if (var->is_in_shader_storage_block() &&
> var->type->is_unsized_array()) {
> +var->data.from_ssbo_unsized_array = true;
> +if (!is_unsized_array_last_element(var)) {
> + _mesa_glsl_error(&loc, state, "unsized array `%s' should be
> the"
> + " last member of a shader storage block",
> + var->name);
> +}
> + }
The existing comment above this is misleading I think its a copy and
paste error from the non instance name block code below.
Anyway to get to the point here you are testing if the block itself is
an unsized array not the member inside the block.
Also when gles both this test and the test below should throw an error
when there is an unsized array member but its not in a shader storage
block.
"If an array is declared as the last member of a shader storage block
and the size is not specified at compile-time, it is sized at run-time.
In all other cases, arrays are sized only at compile-time."
> +
> state->symbols->add_variable(var);
> instructions->push_tail(var);
>}
> @@ -5900,6 +5922,16 @@ ast_interface_block::hir(exec_list *instructions,
> var->data.explicit_binding = this->layout.flags.q.explicit_binding;
> var->data.binding = this->layout.binding;
>
> + if (var->data.mode == ir_var_shader_storage &&
> var->type->is_unsized_array()) {
> +var->data.from_ssbo_unsized_array = true;
> +if (var->is_in_shader_storage_block() &&
> +!is_unsized_array_last_element(var)) {
> + _mesa_glsl_error(&loc, state, "unsized array `%s' should be
> the"
> + " last member of a shader storage block",
> + var->name);
> +}
> + }
> +
> state->symbols->add_variable(var);
> instructions->push_tail(var);
>}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: store full array type in gl_uniform_storage
I've created a new piglit test to confirm this fixes a bug in
_mesa_sampler_uniforms_pipeline_are_valid()
http://lists.freedesktop.org/archives/piglit/2015-June/016270.html
I'll update the commit message to:
"Previously only the type of a single array element was stored.
_mesa_sampler_uniforms_pipeline_are_valid() was expecting to get the
array type so this fixes a bug with validating arrays of samplers in
SSO.
This change will also useful for implementing arrays of arrays support
in glGetUniformLocation()."
I guess this could also be a stable candidate.
On Mon, 2015-06-08 at 18:58 +1000, Timothy Arceri wrote:
> Previously only the type of a single array element
> was stored.
>
> _mesa_sampler_uniforms_pipeline_are_valid() was expecting to get the array
> type so this probably fixes a bug there.
> However the main reason for doing this is to use the array type for
> implementing arrays of arrays in glGetUniformLocation() in an upcoming
> patch series.
> ---
> src/glsl/ir_uniform.h | 5 +-
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +-
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 5 +-
> src/mesa/main/shader_query.cpp | 2 +-
> src/mesa/main/uniform_query.cpp| 64
> ++
> src/mesa/program/ir_to_mesa.cpp| 7 +--
> 6 files changed, 46 insertions(+), 40 deletions(-)
>
> diff --git a/src/glsl/ir_uniform.h b/src/glsl/ir_uniform.h
> index e1b8014..07dd3c3 100644
> --- a/src/glsl/ir_uniform.h
> +++ b/src/glsl/ir_uniform.h
> @@ -91,9 +91,8 @@ struct gl_opaque_uniform_index {
>
> struct gl_uniform_storage {
> char *name;
> - /** Type of this uniform data stored.
> -*
> -* In the case of an array, it's the type of a single array element.
> + /**
> +* Type of this uniform
> */
> const struct glsl_type *type;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 5d3501c..6b669c2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -220,6 +220,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
> unsigned index = var->data.driver_location;
> for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
>struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u];
> + const glsl_type *type = storage->type->without_array();
>
>if (storage->builtin)
>continue;
> @@ -231,7 +232,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
> continue;
>}
>
> - unsigned slots = storage->type->component_slots();
> + unsigned slots = type->component_slots();
>if (storage->array_elements)
> slots *= storage->array_elements;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 242d007..e5874ca 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -685,6 +685,7 @@ vec4_visitor::setup_uniform_values(ir_variable *ir)
> */
> for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
>struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u];
> + const glsl_type *type = storage->type->without_array();
>
>if (storage->builtin)
> continue;
> @@ -698,11 +699,11 @@ vec4_visitor::setup_uniform_values(ir_variable *ir)
>
>gl_constant_value *components = storage->storage;
>unsigned vector_count = (MAX2(storage->array_elements, 1) *
> - storage->type->matrix_columns);
> + type->matrix_columns);
>
>for (unsigned s = 0; s < vector_count; s++) {
> assert(uniforms < uniform_array_size);
> - uniform_vector_size[uniforms] = storage->type->vector_elements;
> + uniform_vector_size[uniforms] = type->vector_elements;
>
> int i;
> for (i = 0; i < uniform_vector_size[uniforms]; i++) {
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index a6246a3..807a95c 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -953,7 +953,7 @@ _mesa_program_resource_prop(struct gl_shader_program
> *shProg,
> case GL_TYPE:
>switch (res->Type) {
>case GL_UNIFORM:
> - *val = RESOURCE_UNI(res)->type->gl_type;
> + *val = RESOURCE_UNI(res)->type->with
[Mesa-dev] [PATCH] mesa: fix active sampler conflict validation for arrays
The type stored in gl_uniform_storage is the type of a single array
element not the array type so size was always 1
---
src/mesa/main/uniform_query.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
Fixes new piglit test:
http://lists.freedesktop.org/archives/piglit/2015-June/016270.html
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index cab5083..cd558ba 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -1101,15 +1101,14 @@ _mesa_sampler_uniforms_pipeline_are_valid(struct
gl_pipeline_object *pipeline)
for (unsigned i = 0; i < shProg[idx]->NumUniformStorage; i++) {
const struct gl_uniform_storage *const storage =
&shProg[idx]->UniformStorage[i];
- const glsl_type *const t = (storage->type->is_array())
-? storage->type->fields.array : storage->type;
+ const glsl_type *const t = storage->type;
if (!t->is_sampler())
continue;
active_samplers++;
- const unsigned count = MAX2(1, storage->type->array_size());
+ const unsigned count = MAX2(1, storage->array_elements);
for (unsigned j = 0; j < count; j++) {
const unsigned unit = storage->storage[j].i;
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: store full array type in gl_uniform_storage
I've sent a smaller fix for this bug, will save this change for an
upcoming AoA patch series.
On Wed, 2015-06-17 at 22:24 +1000, Timothy Arceri wrote:
> I've created a new piglit test to confirm this fixes a bug in
> _mesa_sampler_uniforms_pipeline_are_valid()
>
> http://lists.freedesktop.org/archives/piglit/2015-June/016270.html
>
> I'll update the commit message to:
>
> "Previously only the type of a single array element was stored.
>
> _mesa_sampler_uniforms_pipeline_are_valid() was expecting to get the
> array type so this fixes a bug with validating arrays of samplers in
> SSO.
>
> This change will also useful for implementing arrays of arrays support
> in glGetUniformLocation()."
>
>
> I guess this could also be a stable candidate.
>
>
> On Mon, 2015-06-08 at 18:58 +1000, Timothy Arceri wrote:
> > Previously only the type of a single array element
> > was stored.
> >
> > _mesa_sampler_uniforms_pipeline_are_valid() was expecting to get the array
> > type so this probably fixes a bug there.
> > However the main reason for doing this is to use the array type for
> > implementing arrays of arrays in glGetUniformLocation() in an upcoming
> > patch series.
> > ---
> > src/glsl/ir_uniform.h | 5 +-
> > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +-
> > src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 5 +-
> > src/mesa/main/shader_query.cpp | 2 +-
> > src/mesa/main/uniform_query.cpp| 64
> > ++
> > src/mesa/program/ir_to_mesa.cpp| 7 +--
> > 6 files changed, 46 insertions(+), 40 deletions(-)
> >
> > diff --git a/src/glsl/ir_uniform.h b/src/glsl/ir_uniform.h
> > index e1b8014..07dd3c3 100644
> > --- a/src/glsl/ir_uniform.h
> > +++ b/src/glsl/ir_uniform.h
> > @@ -91,9 +91,8 @@ struct gl_opaque_uniform_index {
> >
> > struct gl_uniform_storage {
> > char *name;
> > - /** Type of this uniform data stored.
> > -*
> > -* In the case of an array, it's the type of a single array element.
> > + /**
> > +* Type of this uniform
> > */
> > const struct glsl_type *type;
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > index 5d3501c..6b669c2 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > @@ -220,6 +220,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
> > unsigned index = var->data.driver_location;
> > for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
> >struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u];
> > + const glsl_type *type = storage->type->without_array();
> >
> >if (storage->builtin)
> >continue;
> > @@ -231,7 +232,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
> > continue;
> >}
> >
> > - unsigned slots = storage->type->component_slots();
> > + unsigned slots = type->component_slots();
> >if (storage->array_elements)
> > slots *= storage->array_elements;
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > index 242d007..e5874ca 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > @@ -685,6 +685,7 @@ vec4_visitor::setup_uniform_values(ir_variable *ir)
> > */
> > for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
> >struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u];
> > + const glsl_type *type = storage->type->without_array();
> >
> >if (storage->builtin)
> > continue;
> > @@ -698,11 +699,11 @@ vec4_visitor::setup_uniform_values(ir_variable *ir)
> >
> >gl_constant_value *components = storage->storage;
> >unsigned vector_count = (MAX2(storage->array_elements, 1) *
> > - storage->type->matrix_columns);
> > + type->matrix_columns);
> >
> >for (unsigned s = 0; s < vector_count; s++) {
> > assert(uniforms < uniform_array_size);
> > - uniform_vector_size[uniforms] = storage->type->vector_elements;
> > + uniform
Re: [Mesa-dev] [PATCH 2/2] glsl: add version checks to conditionals for builtin variable enablement
Reviewed-by: Timothy Arceri
On Wed, 2015-06-17 at 15:15 -0400, Ilia Mirkin wrote:
> A number of builtin variables have checks based on the extension being
> enabled, but were missing enablement via a higher GLSL version.
>
> Signed-off-by: Ilia Mirkin
> Cc: "10.5 10.6"
> ---
> src/glsl/builtin_variables.cpp | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
> index 6806aa1..c52b252 100644
> --- a/src/glsl/builtin_variables.cpp
> +++ b/src/glsl/builtin_variables.cpp
> @@ -876,9 +876,9 @@ void
> builtin_variable_generator::generate_gs_special_vars()
> {
> add_output(VARYING_SLOT_LAYER, int_t, "gl_Layer");
> - if (state->ARB_viewport_array_enable)
> + if (state->is_version(410, 0) || state->ARB_viewport_array_enable)
>add_output(VARYING_SLOT_VIEWPORT, int_t, "gl_ViewportIndex");
> - if (state->ARB_gpu_shader5_enable)
> + if (state->is_version(400, 0) || state->ARB_gpu_shader5_enable)
>add_system_value(SYSTEM_VALUE_INVOCATION_ID, int_t, "gl_InvocationID");
>
> /* Although gl_PrimitiveID appears in tessellation control and
> tessellation
> @@ -946,7 +946,7 @@ builtin_variable_generator::generate_fs_special_vars()
> var->enable_extension_warning("GL_AMD_shader_stencil_export");
> }
>
> - if (state->ARB_sample_shading_enable) {
> + if (state->is_version(400, 0) || state->ARB_sample_shading_enable) {
>add_system_value(SYSTEM_VALUE_SAMPLE_ID, int_t, "gl_SampleID");
>add_system_value(SYSTEM_VALUE_SAMPLE_POS, vec2_t, "gl_SamplePosition");
>/* From the ARB_sample_shading specification:
> @@ -959,11 +959,11 @@ builtin_variable_generator::generate_fs_special_vars()
>add_output(FRAG_RESULT_SAMPLE_MASK, array(int_t, 1), "gl_SampleMask");
> }
>
> - if (state->ARB_gpu_shader5_enable) {
> + if (state->is_version(400, 0) || state->ARB_gpu_shader5_enable) {
>add_system_value(SYSTEM_VALUE_SAMPLE_MASK_IN, array(int_t, 1),
> "gl_SampleMaskIn");
> }
>
> - if (state->ARB_fragment_layer_viewport_enable) {
> + if (state->is_version(430, 0) ||
> state->ARB_fragment_layer_viewport_enable) {
>add_input(VARYING_SLOT_LAYER, int_t, "gl_Layer");
>add_input(VARYING_SLOT_VIEWPORT, int_t, "gl_ViewportIndex");
> }
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: add GL_PROGRAM_PIPELINE support in KHR_debug calls
On Wed, 2015-06-17 at 23:02 -0400, Ilia Mirkin wrote:
> This was apparently missed when KHR_debug or ARB_sso support was added.
SSO was still missing when I added KHR_debug, but this should have been
picked up by the object label piglit test I wrote. However the test was
broken, it failed to fail :(
I've sent a patch to fix this:
http://lists.freedesktop.org/archives/piglit/2015-June/016281.html
Your patch looks good to me and passes the fix piglit test.
Reviewed-by: Timothy Arceri
> Add label support to pipeline objects just like all the other
> debug-related objects.
>
> Signed-off-by: Ilia Mirkin
> Cc: "10.5 10.6"
> ---
> src/mesa/main/mtypes.h | 2 ++
> src/mesa/main/objectlabel.c | 10 --
> src/mesa/main/pipelineobj.c | 21 +++--
> src/mesa/main/pipelineobj.h | 3 +++
> 4 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index ffa7f0c..983b9dc 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2815,6 +2815,8 @@ struct gl_pipeline_object
>
> mtx_t Mutex;
>
> + GLchar *Label; /**< GL_KHR_debug */
> +
> /**
> * Programs used for rendering
> *
> diff --git a/src/mesa/main/objectlabel.c b/src/mesa/main/objectlabel.c
> index aecb5b1..5626054 100644
> --- a/src/mesa/main/objectlabel.c
> +++ b/src/mesa/main/objectlabel.c
> @@ -30,6 +30,7 @@
> #include "enums.h"
> #include "fbobject.h"
> #include "objectlabel.h"
> +#include "pipelineobj.h"
> #include "queryobj.h"
> #include "samplerobj.h"
> #include "shaderobj.h"
> @@ -214,8 +215,13 @@ get_label_pointer(struct gl_context *ctx, GLenum
> identifier, GLuint name,
>}
>break;
> case GL_PROGRAM_PIPELINE:
> - /* requires GL 4.2 */
> - goto invalid_enum;
> + {
> + struct gl_pipeline_object *pipe =
> +_mesa_lookup_pipeline_object(ctx, name);
> + if (pipe)
> +labelPtr = &pipe->Label;
> + }
> + break;
> default:
>goto invalid_enum;
> }
> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
> index b4795ff..279ae20 100644
> --- a/src/mesa/main/pipelineobj.c
> +++ b/src/mesa/main/pipelineobj.c
> @@ -65,6 +65,7 @@ _mesa_delete_pipeline_object(struct gl_context *ctx,
>
> _mesa_reference_shader_program(ctx, &obj->ActiveProgram, NULL);
> mtx_destroy(&obj->Mutex);
> + free(obj->Label);
> ralloc_free(obj);
> }
>
> @@ -136,8 +137,8 @@ _mesa_free_pipeline_data(struct gl_context *ctx)
> * a non-existent ID. The spec defines ID 0 as being technically
> * non-existent.
> */
> -static inline struct gl_pipeline_object *
> -lookup_pipeline_object(struct gl_context *ctx, GLuint id)
> +struct gl_pipeline_object *
> +_mesa_lookup_pipeline_object(struct gl_context *ctx, GLuint id)
> {
> if (id == 0)
>return NULL;
> @@ -225,7 +226,7 @@ _mesa_UseProgramStages(GLuint pipeline, GLbitfield
> stages, GLuint program)
> {
> GET_CURRENT_CONTEXT(ctx);
>
> - struct gl_pipeline_object *pipe = lookup_pipeline_object(ctx, pipeline);
> + struct gl_pipeline_object *pipe = _mesa_lookup_pipeline_object(ctx,
> pipeline);
> struct gl_shader_program *shProg = NULL;
> GLbitfield any_valid_stages;
>
> @@ -337,7 +338,7 @@ _mesa_ActiveShaderProgram(GLuint pipeline, GLuint program)
> {
> GET_CURRENT_CONTEXT(ctx);
> struct gl_shader_program *shProg = NULL;
> - struct gl_pipeline_object *pipe = lookup_pipeline_object(ctx, pipeline);
> + struct gl_pipeline_object *pipe = _mesa_lookup_pipeline_object(ctx,
> pipeline);
>
> if (program != 0) {
>shProg = _mesa_lookup_shader_program_err(ctx, program,
> @@ -399,7 +400,7 @@ _mesa_BindProgramPipeline(GLuint pipeline)
> */
> if (pipeline) {
>/* non-default pipeline object */
> - newObj = lookup_pipeline_object(ctx, pipeline);
> + newObj = _mesa_lookup_pipeline_object(ctx, pipeline);
>if (!newObj) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "glBindProgramPipeline(non-gen name)");
> @@ -468,7 +469,7 @@ _mesa_DeleteProgramPipelines(GLsizei n, const GLuint
> *pipelines)
>
> for (i = 0; i < n; i++) {
>struct gl_pipeline_object *obj =
> - lookup_pipeline_object(ctx, pipelines[i]);
> + _mesa_lookup_pipeline_object(ctx, pipelines[i]);
>
>if (obj) {
> assert(obj->Name == pipelines[i]);
> @@ -568,7 +569,7 @@ _me
[Mesa-dev] [PATCH] docs: update developer info
Just link directly to the piglit repo the old link has outdated information. Add note about updating patchwork when sending patch revisions. --- docs/devinfo.html | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/devinfo.html b/docs/devinfo.html index 0da18b9..6779ab4 100644 --- a/docs/devinfo.html +++ b/docs/devinfo.html @@ -244,7 +244,7 @@ to update the tests themselves. Whenever possible and applicable, test the patch with -http://people.freedesktop.org/~nh/piglit/";>Piglit to +http://cgit.freedesktop.org/piglit";>Piglit to check for regressions. @@ -266,6 +266,12 @@ re-sending the whole series). Using --in-reply-to makes it harder for reviewers to accidentally review old patches. + +When submitting follow-up patches you should also login to +https://patchwork.freedesktop.org";>patchwork and change the +state of your old patches to Superseded. + + Reviewing Patches -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/19] glsl: calculate component size for arrays of arrays when varying packing disabled
Signed-off-by: Timothy Arceri
Reviewed-by: Ilia Mirkin
---
src/glsl/link_varyings.cpp | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 7b2d4bd..6cb55b3 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -876,9 +876,18 @@ varying_matches::record(ir_variable *producer_var,
ir_variable *consumer_var)
this->matches[this->num_matches].packing_order
= this->compute_packing_order(var);
if (this->disable_varying_packing) {
- unsigned slots = var->type->is_array()
- ? (var->type->length * var->type->fields.array->matrix_columns)
- : var->type->matrix_columns;
+ unsigned slots;
+ if (var->type->is_array()) {
+ const glsl_type *type = var->type;
+ slots = 1;
+ while (type->is_array()) {
+slots *= type->length;
+type = type->fields.array;
+ }
+ slots *= type->matrix_columns;
+ } else {
+ slots = var->type->matrix_columns;
+ }
this->matches[this->num_matches].num_components = 4 * slots;
} else {
this->matches[this->num_matches].num_components
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/19] glsl: dont allow gl_PerVertex to be redeclared as an array of arrays
V2: move single dimensionial array detection into a helper
Signed-off-by: Timothy Arceri
---
src/glsl/ast.h | 8
src/glsl/ast_to_hir.cpp | 3 ++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index ef74e51..3f67907 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -328,6 +328,14 @@ public:
array_dimensions.push_tail(&dim->link);
}
+ bool is_single_dimension()
+ {
+ return (this->is_unsized_array && this->array_dimensions.is_empty()) ||
+ (!this->is_unsized_array &&
+ this->array_dimensions.tail_pred->prev != NULL &&
+ this->array_dimensions.tail_pred->prev->is_head_sentinel());
+ }
+
virtual void print(void) const;
/* If true, this means that the array has an unsized outermost dimension. */
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 259e01e..f1c3e4a 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -5672,7 +5672,8 @@ ast_interface_block::hir(exec_list *instructions,
_mesa_shader_stage_to_string(state->stage));
}
if (this->instance_name == NULL ||
- strcmp(this->instance_name, "gl_in") != 0 ||
this->array_specifier == NULL) {
+ strcmp(this->instance_name, "gl_in") != 0 ||
this->array_specifier == NULL ||
+ !this->array_specifier->is_single_dimension()) {
_mesa_glsl_error(&loc, state,
"gl_PerVertex input must be redeclared as "
"gl_in[]");
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/19] glsl: avoid hitting assert for arrays of arrays
Also add TODO comment about adding proper support Signed-off-by: Timothy Arceri --- src/glsl/ir_set_program_inouts.cpp | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/glsl/ir_set_program_inouts.cpp b/src/glsl/ir_set_program_inouts.cpp index b968a1e..01d84fe 100644 --- a/src/glsl/ir_set_program_inouts.cpp +++ b/src/glsl/ir_set_program_inouts.cpp @@ -184,6 +184,12 @@ ir_set_program_inouts_visitor::try_mark_partial_variable(ir_variable *var, type = type->fields.array; } + /* TODO: implement proper arrays of arrays support +* for now let the caller mark whole variable as used. +*/ + if (type->is_array() && type->fields.array->is_array()) + return false; + /* The code below only handles: * * - Indexing into matrices -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/19] glsl: update types for unsized array members
Assigns a new array type based on the max access of
unsized array members.
Reviewed-by: Ilia Mirkin
---
src/glsl/linker.cpp | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 9978380..3494464 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1217,8 +1217,7 @@ public:
resize_interface_members(var->type->fields.array,
var->get_max_ifc_array_access());
var->change_interface_type(new_type);
-var->type =
- glsl_type::get_array_instance(new_type, var->type->length);
+var->type = update_interface_members_array(var->type, new_type);
}
} else if (const glsl_type *ifc_type = var->get_interface_type()) {
/* Store a pointer to the variable in the unnamed_interfaces
@@ -1266,6 +1265,21 @@ private:
}
}
+ static const glsl_type *
+ update_interface_members_array(const glsl_type *type,
+ const glsl_type *new_interface_type)
+ {
+ const glsl_type *element_type = type->fields.array;
+ if (element_type->is_array()) {
+ const glsl_type *new_array_type =
+update_interface_members_array(element_type, new_interface_type);
+ return glsl_type::get_array_instance(new_array_type, type->length);
+ } else {
+ return glsl_type::get_array_instance(new_interface_type,
+ type->length);
+ }
+ }
+
/**
* Determine whether the given interface type contains unsized arrays (if
* it doesn't, array_sizing_visitor doesn't need to process it).
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/19] glsl: Add support for linking uniform arrays of arrays
---
src/glsl/link_uniform_initializers.cpp | 51 --
src/glsl/link_uniforms.cpp | 28 +++
2 files changed, 52 insertions(+), 27 deletions(-)
diff --git a/src/glsl/link_uniform_initializers.cpp
b/src/glsl/link_uniform_initializers.cpp
index 204acfa..6164bdd 100644
--- a/src/glsl/link_uniform_initializers.cpp
+++ b/src/glsl/link_uniform_initializers.cpp
@@ -100,6 +100,38 @@ copy_constant_to_storage(union gl_constant_value *storage,
}
void
+copy_constant_array_to_storage(struct gl_uniform_storage *const storage,
+ const ir_constant *val,
+ unsigned int *idx,
+ unsigned int *array_elements,
+ unsigned int boolean_true)
+{
+ if (val->type->fields.array->is_array()) {
+ for (unsigned int i = 0; i < val->type->length; i++) {
+ copy_constant_array_to_storage(storage, val->array_elements[i],
+idx, array_elements, boolean_true);
+ }
+ } else {
+ const enum glsl_base_type base_type =
+val->array_elements[0]->type->base_type;
+ const unsigned int elements = val->array_elements[0]->type->components();
+ unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1;
+ unsigned int length = MIN2(val->type->length,
+ (storage->array_elements - *array_elements));
+
+ for (unsigned int i = 0; i < length; i++) {
+ copy_constant_to_storage(& storage->storage[*idx],
+ val->array_elements[i],
+ base_type,
+ elements,
+ boolean_true);
+ *idx += elements * dmul;
+ *array_elements = *array_elements + 1;
+ }
+ }
+}
+
+void
set_sampler_binding(gl_shader_program *prog, const char *name, int binding)
{
struct gl_uniform_storage *const storage =
@@ -178,7 +210,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program
*prog,
field_constant = (ir_constant *)field_constant->next;
}
return;
- } else if (type->is_array() && type->fields.array->is_record()) {
+ } else if (type->without_array()->is_record()) {
const glsl_type *const element_type = type->fields.array;
for (unsigned int i = 0; i < type->length; i++) {
@@ -201,22 +233,11 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program
*prog,
}
if (val->type->is_array()) {
- const enum glsl_base_type base_type =
-val->array_elements[0]->type->base_type;
- const unsigned int elements = val->array_elements[0]->type->components();
unsigned int idx = 0;
- unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1;
+ unsigned int array_elements = 0;
- assert(val->type->length >= storage->array_elements);
- for (unsigned int i = 0; i < storage->array_elements; i++) {
-copy_constant_to_storage(& storage->storage[idx],
- val->array_elements[i],
- base_type,
- elements,
- boolean_true);
-
-idx += elements * dmul;
- }
+ copy_constant_array_to_storage(storage, val, &idx,
+ &array_elements, boolean_true);
} else {
copy_constant_to_storage(storage->storage,
val,
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 11ae06f..6b6b197 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -49,8 +49,8 @@ values_for_type(const glsl_type *type)
{
if (type->is_sampler()) {
return 1;
- } else if (type->is_array() && type->fields.array->is_sampler()) {
- return type->array_size();
+ } else if (type->is_array()) {
+ return type->array_size() * values_for_type(type->fields.array);
} else {
return type->component_slots();
}
@@ -71,6 +71,7 @@ void
program_resource_visitor::process(ir_variable *var)
{
const glsl_type *t = var->type;
+ const glsl_type *t_without_array = var->type->without_array();
const bool row_major =
var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
@@ -136,7 +137,7 @@ program_resource_visitor::process(ir_variable *var)
*/
recursion(var->type, &name, strlen(name), row_major, NULL, false);
ralloc_free(name);
- } else if (t->without_array()->is_record()) {
+ } else if (t_without_array->is_record()) {
char *name = ralloc_strdup(NULL, var->name);
recursion(var->type, &name, strlen(name), row_major, NULL, false);
ralloc_free(name);
@@ -144,8 +145,8 @@ program_resource_visitor::process(ir_variable *var)
char *name = ralloc_strdup(NULL, var->type->name);
recursion(var->type, &name, strlen(name), row_major, NULL, false);
ralloc
[Mesa-dev] [PATCH 09/19] glsl: add helper for calculating size of AoA
---
src/glsl/glsl_types.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
index f54a939..c48977c 100644
--- a/src/glsl/glsl_types.h
+++ b/src/glsl/glsl_types.h
@@ -537,6 +537,25 @@ struct glsl_type {
}
/**
+* Return the total number of elements in an array including the elements
+* in arrays of arrays.
+*/
+ unsigned arrays_of_arrays_size() const
+ {
+ if (!is_array())
+ return -1;
+
+ unsigned size = length;
+ const glsl_type *base_type = fields.array;
+
+ while (base_type->is_array()) {
+ size = size * base_type->length;
+ base_type = base_type->fields.array;
+ }
+ return size;
+ }
+
+ /**
* Return the amount of atomic counter storage required for a type.
*/
unsigned atomic_size() const
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/19] glsl: update assert to support arrays of arrays
Reviewed-by: Ilia Mirkin
---
src/glsl/glsl_types.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index f675e90..3547561 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -1086,7 +1086,8 @@ glsl_type::std140_base_alignment(bool row_major) const
this->fields.array->is_matrix()) {
return MAX2(this->fields.array->std140_base_alignment(row_major), 16);
} else {
-assert(this->fields.array->is_record());
+assert(this->fields.array->is_record() ||
+this->fields.array->is_array());
return this->fields.array->std140_base_alignment(row_major);
}
}
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/19] nir: add support for sampler AoA with const index
---
src/glsl/nir/nir_lower_samplers.cpp | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/glsl/nir/nir_lower_samplers.cpp
b/src/glsl/nir/nir_lower_samplers.cpp
index c450198..a79b093 100644
--- a/src/glsl/nir/nir_lower_samplers.cpp
+++ b/src/glsl/nir/nir_lower_samplers.cpp
@@ -60,9 +60,23 @@ lower_sampler(nir_tex_instr *instr, const struct
gl_shader_program *shader_progr
if (instr->sampler == NULL)
return;
+ /* Get number of array dimensions */
+ unsigned num_dimensions = 1;
+ for (nir_deref *deref = &instr->sampler->deref;
+deref->child; deref = deref->child) {
+ if (deref->child->deref_type == nir_deref_type_array) {
+ nir_deref_array *deref_array = nir_deref_as_array(deref->child);
+ if (deref_array->deref.child &&
+!deref->child->type->without_array()->is_record()) {
+num_dimensions++;
+ }
+ }
+ }
+
/* Get the name and the offset */
instr->sampler_index = 0;
char *name = ralloc_strdup(mem_ctx, instr->sampler->var->name);
+ unsigned curr_dim = num_dimensions;
for (nir_deref *deref = &instr->sampler->deref;
deref->child; deref = deref->child) {
@@ -78,8 +92,10 @@ lower_sampler(nir_tex_instr *instr, const struct
gl_shader_program *shader_progr
deref_array->deref_array_type == nir_deref_array_type_direct ?
deref_array->base_offset : 0);
} else {
-assert(deref->child->type->base_type == GLSL_TYPE_SAMPLER);
-instr->sampler_index = deref_array->base_offset;
+/* calculate offset allowing for for arrays of arrays */
+instr->sampler_index +=
+ pow(glsl_get_length(deref->type), --curr_dim) *
+ deref_array->base_offset;
}
/* XXX: We're assuming here that the indirect is the last array
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/19] nir: support uniform sampler AoA lookup
To do this we make sure to only append the array subscript
to structs.
---
src/glsl/nir/nir_lower_samplers.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/glsl/nir/nir_lower_samplers.cpp
b/src/glsl/nir/nir_lower_samplers.cpp
index 7a0b0a0..c450198 100644
--- a/src/glsl/nir/nir_lower_samplers.cpp
+++ b/src/glsl/nir/nir_lower_samplers.cpp
@@ -72,7 +72,8 @@ lower_sampler(nir_tex_instr *instr, const struct
gl_shader_program *shader_progr
assert(deref_array->deref_array_type !=
nir_deref_array_type_wildcard);
- if (deref_array->deref.child) {
+ if (deref_array->deref.child &&
+deref->child->type->without_array()->is_record()) {
ralloc_asprintf_append(&name, "[%u]",
deref_array->deref_array_type == nir_deref_array_type_direct ?
deref_array->base_offset : 0);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/19] glsl: validate binding qualifier for AoA
--- src/glsl/ast_to_hir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index f5e3570..de13060 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -2056,7 +2056,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state, } const struct gl_context *const ctx = state->ctx; - unsigned elements = type->is_array() ? type->length : 1; + unsigned elements = type->is_array() ? type->arrays_of_arrays_size() : 1; unsigned max_index = qual->binding + elements - 1; const glsl_type *base_type = type->without_array(); -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 13/19] glsl: support uniform sampler AoA lookup
To do this we make sure to only append the array subscript
to structs.
---
src/mesa/program/sampler.cpp | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/mesa/program/sampler.cpp b/src/mesa/program/sampler.cpp
index ea3024d..34eee6d 100644
--- a/src/mesa/program/sampler.cpp
+++ b/src/mesa/program/sampler.cpp
@@ -38,14 +38,12 @@
class get_sampler_name : public ir_hierarchical_visitor
{
public:
- get_sampler_name(ir_dereference *last,
- struct gl_shader_program *shader_program)
+ get_sampler_name(struct gl_shader_program *shader_program)
{
this->mem_ctx = ralloc_context(NULL);
this->shader_program = shader_program;
this->name = NULL;
this->offset = 0;
- this->last = last;
}
~get_sampler_name()
@@ -86,7 +84,7 @@ public:
"and is unlikely to be supported for 1.10 in Mesa.\n");
i = 0;
}
- if (ir != last) {
+ if (ir->type->without_array()->is_record()) {
this->name = ralloc_asprintf(mem_ctx, "%s[%d]", name, i);
} else {
offset = i;
@@ -98,7 +96,6 @@ public:
const char *name;
void *mem_ctx;
int offset;
- ir_dereference *last;
};
@@ -107,7 +104,7 @@ _mesa_get_sampler_uniform_value(class ir_dereference
*sampler,
struct gl_shader_program *shader_program,
const struct gl_program *prog)
{
- get_sampler_name getname(sampler, shader_program);
+ get_sampler_name getname(shader_program);
GLuint shader = _mesa_program_enum_to_shader_stage(prog->Target);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] ARB_arrays_of_arrays GLSL ES
Hi all, The restrictions in ES make the extension easier to implement so I thought I'd try get this stuff reviewed an committed before finishing up the full extension. The bits that I'm still working on for the desktop version are AoA inputs outputs, and interface blocks. The only thing I know is definatly missing in this series for ES is support for indirect indexing of samplers, but that didn't seem like something that should hold up the series. Once the SSBO series lands (with a patch that restricts unsized arrays) then all the AoA ES conformance tests will pass. There are already a bunch of piglit tests in git but I've just sent a series with all the patches still waiting review here: http://lists.freedesktop.org/archives/piglit/2015-June/016312.html I haven't made a patch marking this as done yet because currently the i965 backend takes a very long time trying to optimise some of the conformance tests. They still pass but they are taking 15-minutes+ just to compile so this really needs to be sorted out first. If someone with more knowledge in this area than me wants to take a look at this I would be greatful for being pointed in the right direction. If useful the series is in the 'gles4' branch of the repo here: https://github.com/tarceri/Mesa_arrays_of_arrays.git Thanks, Tim ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/19] glsl: allow precision qualifiers for AoA
---
src/glsl/ast_to_hir.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index f1c3e4a..0d3cbac 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3881,9 +3881,7 @@ ast_declarator_list::hir(exec_list *instructions,
* an array of that type.
*/
if (!(this->type->qualifier.precision == ast_precision_none
- || precision_qualifier_allowed(var->type)
- || (var->type->is_array()
- && precision_qualifier_allowed(var->type->fields.array {
+ || precision_qualifier_allowed(var->type->without_array( {
_mesa_glsl_error(&loc, state,
"precision qualifiers apply only to floating point"
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 18/19] glsl: allow AoA to be sized by initializer or constructor
From Section 4.1.9 of the GLSL ES 3.10 spec:
"Arrays are sized either at compile-time or at run-time.
To size an array at compile-time, either the size
must be specified within the brackets as above or
must be inferred from the type of the initializer."
---
src/glsl/ast.h | 21 +++-
src/glsl/ast_array_index.cpp | 7 ++--
src/glsl/ast_function.cpp| 33 +-
src/glsl/ast_to_hir.cpp | 80 ++--
src/glsl/glsl_parser.yy | 11 +++---
5 files changed, 107 insertions(+), 45 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 3f67907..0f9dbf9 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -181,6 +181,7 @@ enum ast_operators {
ast_post_dec,
ast_field_selection,
ast_array_index,
+ ast_unsized_array_dim,
ast_function_call,
@@ -308,16 +309,7 @@ private:
class ast_array_specifier : public ast_node {
public:
- /** Unsized array specifier ([]) */
- explicit ast_array_specifier(const struct YYLTYPE &locp)
- : is_unsized_array(true)
- {
- set_location(locp);
- }
-
- /** Sized array specifier ([dim]) */
ast_array_specifier(const struct YYLTYPE &locp, ast_expression *dim)
- : is_unsized_array(false)
{
set_location(locp);
array_dimensions.push_tail(&dim->link);
@@ -330,19 +322,14 @@ public:
bool is_single_dimension()
{
- return (this->is_unsized_array && this->array_dimensions.is_empty()) ||
- (!this->is_unsized_array &&
- this->array_dimensions.tail_pred->prev != NULL &&
- this->array_dimensions.tail_pred->prev->is_head_sentinel());
+ return this->array_dimensions.tail_pred->prev != NULL &&
+ this->array_dimensions.tail_pred->prev->is_head_sentinel();
}
virtual void print(void) const;
- /* If true, this means that the array has an unsized outermost dimension. */
- bool is_unsized_array;
-
/* This list contains objects of type ast_node containing the
-* sized dimensions only, in outermost-to-innermost order.
+* array dimensions in outermost-to-innermost order.
*/
exec_list array_dimensions;
};
diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
index 752d86f..89c08d8 100644
--- a/src/glsl/ast_array_index.cpp
+++ b/src/glsl/ast_array_index.cpp
@@ -28,13 +28,10 @@
void
ast_array_specifier::print(void) const
{
- if (this->is_unsized_array) {
- printf("[ ] ");
- }
-
foreach_list_typed (ast_node, array_dimension, link,
&this->array_dimensions) {
printf("[ ");
- array_dimension->print();
+ if (((ast_expression*)array_dimension)->oper != ast_unsized_array_dim)
+ array_dimension->print();
printf("] ");
}
}
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 92e26bf..0906040 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -870,6 +870,7 @@ process_array_constructor(exec_list *instructions,
}
bool all_parameters_are_constant = true;
+ const glsl_type *element_type = constructor_type->fields.array;
/* Type cast each parameter and, if possible, fold constants. */
foreach_in_list_safe(ir_rvalue, ir, &actual_parameters) {
@@ -896,12 +897,34 @@ process_array_constructor(exec_list *instructions,
}
}
- if (result->type != constructor_type->fields.array) {
+ if (constructor_type->fields.array->is_unsized_array()) {
+ /* As the inner parameters of the constructor are created without
+ * knowledge of each other we need to check to make sure unsized
+ * parameters of unsized constructors all end up with the same size.
+ *
+ * e.g we make sure to fail for a constructor like this:
+ * vec4[][] a = vec4[][](vec4[](vec4(0.0), vec4(1.0)),
+ * vec4[](vec4(0.0), vec4(1.0), vec4(1.0)),
+ * vec4[](vec4(0.0), vec4(1.0)));
+ */
+ if (element_type->is_unsized_array()) {
+ /* This is the first parameter so just get the type */
+element_type = result->type;
+ } else if (element_type != result->type) {
+_mesa_glsl_error(loc, state, "type error in array constructor: "
+ "expected: %s, found %s",
+ element_type->name,
+ result->type->name);
+return ir_rvalue::error_value(ctx);
+ }
+ } else if (result->type != constructor_type->fields.array) {
_mesa_glsl_error(loc, state, "type error in array constructor: "
"expected: %s, found %s",
constructor_type->fields.array->name,
result->type->name);
return ir_rvalue::error_value(ctx);
+ } else {
+ element_type = result->type;
}
/* Attempt to convert the parameter to a constant valu
[Mesa-dev] [PATCH 17/19] mesa: add AoA support to active sampler conflict validation
---
src/mesa/main/uniform_query.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index f7a5c87..3faf770 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -1113,7 +1113,7 @@ _mesa_sampler_uniforms_pipeline_are_valid(struct
gl_pipeline_object *pipeline)
active_samplers++;
- const unsigned count = MAX2(1, storage->type->array_size());
+ const unsigned count = MAX2(1, storage->array_elements);
for (unsigned j = 0; j < count; j++) {
const unsigned unit = storage->storage[j].i;
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 15/19] mesa: store full array type in gl_uniform_storage
Previously only the type of a single array element
was stored.
_mesa_sampler_uniforms_pipeline_are_valid() was expecting
to get the array type so this fixes a bug there.
However the main reason for doing this is to use the array type
for implementing arrays of arrays in glGetUniformLocation()
---
src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 5 +-
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 5 +-
src/mesa/main/shader_query.cpp | 2 +-
src/mesa/main/uniform_query.cpp| 64 ++
src/mesa/program/ir_to_mesa.cpp| 7 +--
5 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index 59081ea..c67e56b 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -222,6 +222,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
unsigned index = var->data.driver_location;
for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u];
+ const glsl_type *type = storage->type->without_array();
if (storage->builtin)
continue;
@@ -233,7 +234,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
continue;
}
- unsigned slots = storage->type->component_slots();
+ unsigned slots = type->component_slots();
if (storage->array_elements)
slots *= storage->array_elements;
@@ -243,7 +244,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
}
/* Make sure we actually initialized the right amount of stuff here. */
- assert(var->data.driver_location + var->type->component_slots() == index);
+ assert(var->data.driver_location + type->component_slots() == index);
}
void
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 0a76bde..92b6044 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -685,6 +685,7 @@ vec4_visitor::setup_uniform_values(ir_variable *ir)
*/
for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u];
+ const glsl_type *type = storage->type->without_array();
if (storage->builtin)
continue;
@@ -698,11 +699,11 @@ vec4_visitor::setup_uniform_values(ir_variable *ir)
gl_constant_value *components = storage->storage;
unsigned vector_count = (MAX2(storage->array_elements, 1) *
- storage->type->matrix_columns);
+ type->matrix_columns);
for (unsigned s = 0; s < vector_count; s++) {
assert(uniforms < uniform_array_size);
- uniform_vector_size[uniforms] = storage->type->vector_elements;
+ uniform_vector_size[uniforms] = type->vector_elements;
int i;
for (i = 0; i < uniform_vector_size[uniforms]; i++) {
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index a6246a3..807a95c 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -953,7 +953,7 @@ _mesa_program_resource_prop(struct gl_shader_program
*shProg,
case GL_TYPE:
switch (res->Type) {
case GL_UNIFORM:
- *val = RESOURCE_UNI(res)->type->gl_type;
+ *val = RESOURCE_UNI(res)->type->without_array()->gl_type;
return 1;
case GL_PROGRAM_INPUT:
case GL_PROGRAM_OUTPUT:
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index cab5083..c8b0b58 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -320,9 +320,10 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program,
GLint location,
return;
}
+ const glsl_type *uni_type = uni->type->without_array();
{
- unsigned elements = (uni->type->is_sampler())
-? 1 : uni->type->components();
+ unsigned elements = (uni_type->is_sampler())
+? 1 : uni_type->components();
/* Calculate the source base address *BEFORE* modifying elements to
* account for the size of the user's buffer.
@@ -348,14 +349,14 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program,
GLint location,
* just memcpy the data. If the types are not compatible, perform a
* slower convert-and-copy process.
*/
- if (returnType == uni->type->base_type
+ if (returnType == uni_type->base_type
|| ((returnType == GLSL_TYPE_INT
|| returnType == GLSL_TYPE_UINT)
&&
- (uni->type->base_type == GLSL_TYPE_INT
- || uni->type->base_type == GLSL_TYPE_UINT
- || uni->type->base_type == GLSL_TYPE_SAMPLER
- || uni->type->base_type == GLSL_TYPE_IMAGE))) {
+ (uni_type-
[Mesa-dev] [PATCH 19/19] glsl: Allow arrays of arrays in GLSL ES 3.10 and GLSL 4.30
---
src/glsl/ast_to_hir.cpp | 13 -
src/glsl/glsl_parser.yy | 22 ++
src/glsl/glsl_parser_extras.h | 5 +
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 6d2dc2e..81b2765 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1939,12 +1939,15 @@ process_array_type(YYLTYPE *loc, const glsl_type *base,
*
* "Only one-dimensional arrays may be declared."
*/
- if (!state->ARB_arrays_of_arrays_enable) {
+ if (!state->has_arrays_of_arrays()) {
+const char *const requirement = state->es_shader
+ ? "GLSL ES 310"
+ : "GL_ARB_arrays_of_array or GLSL 430";
_mesa_glsl_error(loc, state,
- "invalid array of `%s'"
- "GL_ARB_arrays_of_arrays "
- "required for defining arrays of arrays",
- base->name);
+ "invalid array of `%s' "
+ "%s required for defining arrays of arrays",
+ base->name, requirement);
+
return glsl_type::error_type;
}
}
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 1f08893..49c01f8 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -1856,10 +1856,13 @@ array_specifier:
void *ctx = state;
$$ = $1;
- if (!state->ARB_arrays_of_arrays_enable) {
- _mesa_glsl_error(& @1, state,
- "GL_ARB_arrays_of_arrays "
- "required for defining arrays of arrays");
+ if (!state->has_arrays_of_arrays()) {
+ const char *const requirement = state->es_shader
+? "GLSL ES 310"
+: "GL_ARB_arrays_of_array or GLSL 430";
+ _mesa_glsl_error(& @1, state,
+ "%s required for defining arrays of arrays.",
+ requirement);
}
$$->add_dimension(new(ctx) ast_expression(ast_unsized_array_dim, NULL,
NULL, NULL));
@@ -1868,10 +1871,13 @@ array_specifier:
{
$$ = $1;
- if (!state->ARB_arrays_of_arrays_enable) {
- _mesa_glsl_error(& @1, state,
- "GL_ARB_arrays_of_arrays "
- "required for defining arrays of arrays");
+ if (!state->has_arrays_of_arrays()) {
+ const char *const requirement = state->es_shader
+? "GLSL ES 310"
+: "GL_ARB_arrays_of_array or GLSL 430";
+ _mesa_glsl_error(& @1, state,
+ "%s required for defining arrays of arrays.",
+ requirement);
}
$$->add_dimension($3);
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index 9a0c24e..21182e4 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -190,6 +190,11 @@ struct _mesa_glsl_parse_state {
return true;
}
+ bool has_arrays_of_arrays() const
+ {
+ return ARB_arrays_of_arrays_enable || is_version(430, 310);
+ }
+
bool has_atomic_counters() const
{
return ARB_shader_atomic_counters_enable || is_version(420, 310);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 16/19] glsl: add AoA support to resource name parsing
Updated to parse arrays of arrays and return the correct offset.
We are now also validating the array subscript rather than potentially
returning an offset that will be out of bounds.
---
src/glsl/link_uniforms.cpp | 2 +-
src/glsl/link_varyings.cpp | 7 +--
src/glsl/link_varyings.h| 3 +-
src/glsl/linker.cpp | 108
src/glsl/program.h | 3 +-
src/mesa/main/uniform_query.cpp | 2 +-
6 files changed, 97 insertions(+), 28 deletions(-)
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 6b6b197..38400d9 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -621,7 +621,7 @@ private:
}
this->uniforms[id].name = ralloc_strdup(this->uniforms, name);
- this->uniforms[id].type = base_type;
+ this->uniforms[id].type = type;
this->uniforms[id].initialized = 0;
this->uniforms[id].num_driver_storage = 0;
this->uniforms[id].driver_storage = NULL;
diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 6cb55b3..d97af8f 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -292,7 +292,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program
*prog,
*/
void
tfeedback_decl::init(struct gl_context *ctx, const void *mem_ctx,
- const char *input)
+ const char *input, struct gl_shader_program *shProg)
{
/* We don't have to be pedantic about what is a valid GLSL variable name,
* because any variable with an invalid name can't exist in the IR anyway.
@@ -329,7 +329,8 @@ tfeedback_decl::init(struct gl_context *ctx, const void
*mem_ctx,
/* Parse a declaration. */
const char *base_name_end;
- long subscript = parse_program_resource_name(input, &base_name_end);
+ long subscript = parse_program_resource_name(input, &base_name_end,
+shProg);
this->var_name = ralloc_strndup(mem_ctx, input, base_name_end - input);
if (this->var_name == NULL) {
_mesa_error_no_memory(__func__);
@@ -574,7 +575,7 @@ parse_tfeedback_decls(struct gl_context *ctx, struct
gl_shader_program *prog,
char **varying_names, tfeedback_decl *decls)
{
for (unsigned i = 0; i < num_names; ++i) {
- decls[i].init(ctx, mem_ctx, varying_names[i]);
+ decls[i].init(ctx, mem_ctx, varying_names[i], prog);
if (!decls[i].is_varying())
continue;
diff --git a/src/glsl/link_varyings.h b/src/glsl/link_varyings.h
index afc16a8..443d1ca 100644
--- a/src/glsl/link_varyings.h
+++ b/src/glsl/link_varyings.h
@@ -91,7 +91,8 @@ struct tfeedback_candidate
class tfeedback_decl
{
public:
- void init(struct gl_context *ctx, const void *mem_ctx, const char *input);
+ void init(struct gl_context *ctx, const void *mem_ctx,
+ const char *input, struct gl_shader_program *shProg);
static bool is_same(const tfeedback_decl &x, const tfeedback_decl &y);
bool assign_location(struct gl_context *ctx,
struct gl_shader_program *prog);
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 3494464..66d5706 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -377,18 +377,18 @@ linker_warning(gl_shader_program *prog, const char *fmt,
...)
/**
* Given a string identifying a program resource, break it into a base name
- * and an optional array index in square brackets.
+ * and optional array indices in square brackets.
*
- * If an array index is present, \c out_base_name_end is set to point to the
- * "[" that precedes the array index, and the array index itself is returned
- * as a long.
+ * If array indices are present, \c out_base_name_end is set to point to the
+ * "[" that precedes the first array index, and the an array offset is
+ * returned as a long.
*
* If no array index is present (or if the array index is negative or
* mal-formed), \c out_base_name_end, is set to point to the null terminator
* at the end of the input string, and -1 is returned.
*
- * Only the final array index is parsed; if the string contains other array
- * indices (or structure field accesses), they are left in the base name.
+ * Only the final array indices are parsed; if the string contains other array
+ * indices such as structure field accesses, they are left in the base name.
*
* No attempt is made to check that the base name is properly formed;
* typically the caller will look up the base name in a hash table, so
@@ -396,7 +396,8 @@ linker_warning(gl_shader_program *prog, const char *fmt,
...)
*/
long
parse_program_resource_name(const GLchar *name,
-const GLchar **out_base_name_end)
+const GLchar **out_base_name_end,
+struct gl_shader_program *shProg)
{
/* Section 7.3.1 ("Program Interfaces") of the OpenGL 4.3 spec says:
*
@@
[Mesa-dev] [PATCH 14/19] glsl: add support for sampler AoA with const index
---
src/mesa/program/sampler.cpp | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/mesa/program/sampler.cpp b/src/mesa/program/sampler.cpp
index 34eee6d..0017328 100644
--- a/src/mesa/program/sampler.cpp
+++ b/src/mesa/program/sampler.cpp
@@ -87,7 +87,17 @@ public:
if (ir->type->without_array()->is_record()) {
this->name = ralloc_asprintf(mem_ctx, "%s[%d]", name, i);
} else {
-offset = i;
+ if (ir->type->is_array()) {
+unsigned current_dim = 1;
+const glsl_type *type = ir->type;
+while (type->fields.array->is_array()) {
+ current_dim++;
+ type = type->fields.array;
+}
+offset += pow(ir->type->length, current_dim) * i;
+ } else {
+offset += i;
+ }
}
return visit_continue;
}
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/19] glsl: fix binding validation for interface blocks
V2: fix minor formating issue
---
src/glsl/ast_to_hir.cpp | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 0d3cbac..f5e3570 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2041,10 +2041,10 @@ validate_matrix_layout_for_type(struct
_mesa_glsl_parse_state *state,
static bool
validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
YYLTYPE *loc,
- ir_variable *var,
+ const glsl_type *type,
const ast_type_qualifier *qual)
{
- if (var->data.mode != ir_var_uniform) {
+ if (!qual->flags.q.uniform) {
_mesa_glsl_error(loc, state,
"the \"binding\" qualifier only applies to uniforms");
return false;
@@ -2056,10 +2056,11 @@ validate_binding_qualifier(struct
_mesa_glsl_parse_state *state,
}
const struct gl_context *const ctx = state->ctx;
- unsigned elements = var->type->is_array() ? var->type->length : 1;
+ unsigned elements = type->is_array() ? type->length : 1;
unsigned max_index = qual->binding + elements - 1;
+ const glsl_type *base_type = type->without_array();
- if (var->type->is_interface()) {
+ if (base_type->is_interface()) {
/* UBOs. From page 60 of the GLSL 4.20 specification:
* "If the binding point for any uniform block instance is less than
zero,
* or greater than or equal to the implementation-dependent maximum
@@ -2077,8 +2078,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
ctx->Const.MaxUniformBufferBindings);
return false;
}
- } else if (var->type->is_sampler() ||
- (var->type->is_array() &&
var->type->fields.array->is_sampler())) {
+ } else if (base_type->is_sampler()) {
/* Samplers. From page 63 of the GLSL 4.20 specification:
* "If the binding is less than zero, or greater than or equal to the
* implementation-dependent maximum supported number of units, a
@@ -2095,7 +2095,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
return false;
}
- } else if (var->type->contains_atomic()) {
+ } else if (base_type->contains_atomic()) {
assert(ctx->Const.MaxAtomicBufferBindings <=
MAX_COMBINED_ATOMIC_BUFFERS);
if (unsigned(qual->binding) >= ctx->Const.MaxAtomicBufferBindings) {
_mesa_glsl_error(loc, state, "layout(binding = %d) exceeds the "
@@ -2651,7 +2651,7 @@ apply_type_qualifier_to_variable(const struct
ast_type_qualifier *qual,
}
if (qual->flags.q.explicit_binding &&
- validate_binding_qualifier(state, loc, var, qual)) {
+ validate_binding_qualifier(state, loc, var->type, qual)) {
var->data.explicit_binding = true;
var->data.binding = qual->binding;
}
@@ -5752,6 +5752,8 @@ ast_interface_block::hir(exec_list *instructions,
num_variables,
packing,
this->block_name);
+ if (this->layout.flags.q.explicit_binding)
+ validate_binding_qualifier(state, &loc, block_type, &this->layout);
if (!state->symbols->add_interface(block_type->name, block_type, var_mode))
{
YYLTYPE loc = this->get_location();
@@ -5849,6 +5851,10 @@ ast_interface_block::hir(exec_list *instructions,
"not allowed");
}
+ if (this->layout.flags.q.explicit_binding)
+validate_binding_qualifier(state, &loc, block_array_type,
+ &this->layout);
+
var = new(state) ir_variable(block_array_type,
this->instance_name,
var_mode);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 18/19] glsl: allow AoA to be sized by initializer or constructor
On Sat, 2015-06-20 at 22:33 +1000, Timothy Arceri wrote:
> From Section 4.1.9 of the GLSL ES 3.10 spec:
>
> "Arrays are sized either at compile-time or at run-time.
> To size an array at compile-time, either the size
> must be specified within the brackets as above or
> must be inferred from the type of the initializer."
> ---
> src/glsl/ast.h | 21 +++-
> src/glsl/ast_array_index.cpp | 7 ++--
> src/glsl/ast_function.cpp| 33 +-
> src/glsl/ast_to_hir.cpp | 80
> ++--
> src/glsl/glsl_parser.yy | 11 +++---
> 5 files changed, 107 insertions(+), 45 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 3f67907..0f9dbf9 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -181,6 +181,7 @@ enum ast_operators {
> ast_post_dec,
> ast_field_selection,
> ast_array_index,
> + ast_unsized_array_dim,
>
> ast_function_call,
>
> @@ -308,16 +309,7 @@ private:
>
> class ast_array_specifier : public ast_node {
> public:
> - /** Unsized array specifier ([]) */
> - explicit ast_array_specifier(const struct YYLTYPE &locp)
> - : is_unsized_array(true)
> - {
> - set_location(locp);
> - }
> -
> - /** Sized array specifier ([dim]) */
> ast_array_specifier(const struct YYLTYPE &locp, ast_expression *dim)
> - : is_unsized_array(false)
> {
>set_location(locp);
>array_dimensions.push_tail(&dim->link);
> @@ -330,19 +322,14 @@ public:
>
> bool is_single_dimension()
> {
> - return (this->is_unsized_array && this->array_dimensions.is_empty()) ||
> - (!this->is_unsized_array &&
> - this->array_dimensions.tail_pred->prev != NULL &&
> - this->array_dimensions.tail_pred->prev->is_head_sentinel());
> + return this->array_dimensions.tail_pred->prev != NULL &&
> + this->array_dimensions.tail_pred->prev->is_head_sentinel();
> }
>
> virtual void print(void) const;
>
> - /* If true, this means that the array has an unsized outermost dimension.
> */
> - bool is_unsized_array;
> -
> /* This list contains objects of type ast_node containing the
> -* sized dimensions only, in outermost-to-innermost order.
> +* array dimensions in outermost-to-innermost order.
> */
> exec_list array_dimensions;
> };
> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
> index 752d86f..89c08d8 100644
> --- a/src/glsl/ast_array_index.cpp
> +++ b/src/glsl/ast_array_index.cpp
> @@ -28,13 +28,10 @@
> void
> ast_array_specifier::print(void) const
> {
> - if (this->is_unsized_array) {
> - printf("[ ] ");
> - }
> -
> foreach_list_typed (ast_node, array_dimension, link,
> &this->array_dimensions) {
>printf("[ ");
> - array_dimension->print();
> + if (((ast_expression*)array_dimension)->oper != ast_unsized_array_dim)
> + array_dimension->print();
>printf("] ");
> }
> }
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 92e26bf..0906040 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -870,6 +870,7 @@ process_array_constructor(exec_list *instructions,
> }
>
> bool all_parameters_are_constant = true;
> + const glsl_type *element_type = constructor_type->fields.array;
>
> /* Type cast each parameter and, if possible, fold constants. */
> foreach_in_list_safe(ir_rvalue, ir, &actual_parameters) {
> @@ -896,12 +897,34 @@ process_array_constructor(exec_list *instructions,
>}
>}
>
> - if (result->type != constructor_type->fields.array) {
> + if (constructor_type->fields.array->is_unsized_array()) {
> + /* As the inner parameters of the constructor are created without
> + * knowledge of each other we need to check to make sure unsized
> + * parameters of unsized constructors all end up with the same size.
> + *
> + * e.g we make sure to fail for a constructor like this:
> + * vec4[][] a = vec4[][](vec4[](vec4(0.0), vec4(1.0)),
> + * vec4[](vec4(0.0), vec4(1.0), vec4(1.0)),
> + * vec4[](vec4(0.0), vec4(1.0)));
> + */
> + if (element_type->is_unsized_array()) {
> + /* This is the first parameter so just get the type */
> +element_
Re: [Mesa-dev] [PATCH 16/19] glsl: add AoA support to resource name parsing
Grrr. Not sure how I missed it but this patch breaks transform feedback.
It doesn't seem like a good idea to share this code between the two
codepaths any more, seems like Paul had plains to use it in more places
when it was added but it never happened.
I'll send a version 2 of this patch tomorrow.
On Sat, 2015-06-20 at 22:33 +1000, Timothy Arceri wrote:
> Updated to parse arrays of arrays and return the correct offset.
>
> We are now also validating the array subscript rather than potentially
> returning an offset that will be out of bounds.
> ---
> src/glsl/link_uniforms.cpp | 2 +-
> src/glsl/link_varyings.cpp | 7 +--
> src/glsl/link_varyings.h| 3 +-
> src/glsl/linker.cpp | 108
>
> src/glsl/program.h | 3 +-
> src/mesa/main/uniform_query.cpp | 2 +-
> 6 files changed, 97 insertions(+), 28 deletions(-)
>
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index 6b6b197..38400d9 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -621,7 +621,7 @@ private:
>}
>
>this->uniforms[id].name = ralloc_strdup(this->uniforms, name);
> - this->uniforms[id].type = base_type;
> + this->uniforms[id].type = type;
>this->uniforms[id].initialized = 0;
>this->uniforms[id].num_driver_storage = 0;
>this->uniforms[id].driver_storage = NULL;
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 6cb55b3..d97af8f 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -292,7 +292,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program
> *prog,
> */
> void
> tfeedback_decl::init(struct gl_context *ctx, const void *mem_ctx,
> - const char *input)
> + const char *input, struct gl_shader_program *shProg)
> {
> /* We don't have to be pedantic about what is a valid GLSL variable name,
> * because any variable with an invalid name can't exist in the IR anyway.
> @@ -329,7 +329,8 @@ tfeedback_decl::init(struct gl_context *ctx, const void
> *mem_ctx,
>
> /* Parse a declaration. */
> const char *base_name_end;
> - long subscript = parse_program_resource_name(input, &base_name_end);
> + long subscript = parse_program_resource_name(input, &base_name_end,
> +shProg);
> this->var_name = ralloc_strndup(mem_ctx, input, base_name_end - input);
> if (this->var_name == NULL) {
>_mesa_error_no_memory(__func__);
> @@ -574,7 +575,7 @@ parse_tfeedback_decls(struct gl_context *ctx, struct
> gl_shader_program *prog,
>char **varying_names, tfeedback_decl *decls)
> {
> for (unsigned i = 0; i < num_names; ++i) {
> - decls[i].init(ctx, mem_ctx, varying_names[i]);
> + decls[i].init(ctx, mem_ctx, varying_names[i], prog);
>
>if (!decls[i].is_varying())
> continue;
> diff --git a/src/glsl/link_varyings.h b/src/glsl/link_varyings.h
> index afc16a8..443d1ca 100644
> --- a/src/glsl/link_varyings.h
> +++ b/src/glsl/link_varyings.h
> @@ -91,7 +91,8 @@ struct tfeedback_candidate
> class tfeedback_decl
> {
> public:
> - void init(struct gl_context *ctx, const void *mem_ctx, const char *input);
> + void init(struct gl_context *ctx, const void *mem_ctx,
> + const char *input, struct gl_shader_program *shProg);
> static bool is_same(const tfeedback_decl &x, const tfeedback_decl &y);
> bool assign_location(struct gl_context *ctx,
> struct gl_shader_program *prog);
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 3494464..66d5706 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -377,18 +377,18 @@ linker_warning(gl_shader_program *prog, const char
> *fmt, ...)
>
> /**
> * Given a string identifying a program resource, break it into a base name
> - * and an optional array index in square brackets.
> + * and optional array indices in square brackets.
> *
> - * If an array index is present, \c out_base_name_end is set to point to the
> - * "[" that precedes the array index, and the array index itself is returned
> - * as a long.
> + * If array indices are present, \c out_base_name_end is set to point to the
> + * "[" that precedes the first array index, and the an array offset is
> + * returned as a long.
> *
> * If no array index is present (or if the array index is negative or
> * mal-formed), \c out_base_name_end, is set to point to the null terminator
&
Re: [Mesa-dev] [PATCH 19/19] glsl: Allow arrays of arrays in GLSL ES 3.10 and GLSL 4.30
On Sat, 2015-06-20 at 19:35 -0400, Ilia Mirkin wrote:
> On Sat, Jun 20, 2015 at 8:33 AM, Timothy Arceri wrote:
> > ---
> > src/glsl/ast_to_hir.cpp | 13 -
> > src/glsl/glsl_parser.yy | 22 ++
> > src/glsl/glsl_parser_extras.h | 5 +
> > 3 files changed, 27 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> > index 6d2dc2e..81b2765 100644
> > --- a/src/glsl/ast_to_hir.cpp
> > +++ b/src/glsl/ast_to_hir.cpp
> > @@ -1939,12 +1939,15 @@ process_array_type(YYLTYPE *loc, const glsl_type
> > *base,
> >*
> >* "Only one-dimensional arrays may be declared."
> >*/
> > - if (!state->ARB_arrays_of_arrays_enable) {
> > + if (!state->has_arrays_of_arrays()) {
> > +const char *const requirement = state->es_shader
> > + ? "GLSL ES 310"
> > + : "GL_ARB_arrays_of_array or GLSL 430";
>
> I think everywhere I've seen it's GLSL ES 3.10 and GLSL 4.30. Also, it
> should be arrays_of_array*s*
Thanks for pointing out the missing 's' I'll fix that, however it seems
more common to use GLSL ES 310 and GLSL 430 see glsl_parser_extras.h.
I could only find one instance of "GLSL 4.30" in glsl_parser.yy I
searched from 3.30-4.50
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 19/19] glsl: Allow arrays of arrays in GLSL ES 3.10 and GLSL 4.30
On Sat, 2015-06-20 at 23:57 -0400, Ilia Mirkin wrote:
> On Sat, Jun 20, 2015 at 11:38 PM, Timothy Arceri
> wrote:
> > On Sat, 2015-06-20 at 19:35 -0400, Ilia Mirkin wrote:
> >> On Sat, Jun 20, 2015 at 8:33 AM, Timothy Arceri
> >> wrote:
> >> > ---
> >> > src/glsl/ast_to_hir.cpp | 13 -
> >> > src/glsl/glsl_parser.yy | 22 ++
> >> > src/glsl/glsl_parser_extras.h | 5 +
> >> > 3 files changed, 27 insertions(+), 13 deletions(-)
> >> >
> >> > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> >> > index 6d2dc2e..81b2765 100644
> >> > --- a/src/glsl/ast_to_hir.cpp
> >> > +++ b/src/glsl/ast_to_hir.cpp
> >> > @@ -1939,12 +1939,15 @@ process_array_type(YYLTYPE *loc, const glsl_type
> >> > *base,
> >> >*
> >> >* "Only one-dimensional arrays may be declared."
> >> >*/
> >> > - if (!state->ARB_arrays_of_arrays_enable) {
> >> > + if (!state->has_arrays_of_arrays()) {
> >> > +const char *const requirement = state->es_shader
> >> > + ? "GLSL ES 310"
> >> > + : "GL_ARB_arrays_of_array or GLSL 430";
> >>
> >> I think everywhere I've seen it's GLSL ES 3.10 and GLSL 4.30. Also, it
> >> should be arrays_of_array*s*
> >
> > Thanks for pointing out the missing 's' I'll fix that, however it seems
> > more common to use GLSL ES 310 and GLSL 430 see glsl_parser_extras.h.
> >
> > I could only find one instance of "GLSL 4.30" in glsl_parser.yy I
> > searched from 3.30-4.50
>
> $ git grep -P 'GLSL (ES )?\d{3}'
>
> reveals just a handful of instances:
>
> src/gallium/drivers/freedreno/freedreno_screen.c:
> {"glsl120", FD_DBG_GLSL120,"Temporary flag to force GLSL 120 (rather
> than 130) on a3xx+"},
> src/glsl/glsl_parser_extras.h: const char *const requirement =
> "GL_ARB_gpu_shader5 extension or GLSL 400";
> src/glsl/glsl_parser_extras.h:? "GLSL ES 300"
> src/glsl/glsl_parser_extras.h::
> "GL_ARB_explicit_attrib_location extension or GLSL 330";
> src/glsl/glsl_parser_extras.h:?
> "GL_EXT_separate_shader_objects extension or GLSL ES 310"
> src/glsl/glsl_parser_extras.h::
> "GL_ARB_separate_shader_objects extension or GLSL 420";
> src/glsl/glsl_parser_extras.h:? "GLSL ES 310"
> src/glsl/glsl_parser_extras.h:
> "GL_ARB_explicit_attrib_location or GLSL 330.";
>
> Which IMHO should be fixed. Compare that to
>
> $ git grep -P 'GLSL (ES )?\d{1}\.\d{2}' src | wc -l
> 286
>
> Most of which come from comments... I guess there are just a few that
> end up in user strings:
>
> src/glsl/ast_array_index.cpp: "expressions
> will be forbidden in GLSL 1.30 "
> src/glsl/ast_array_index.cpp:"expressions is
> forbidden in GLSL 1.30 and "
> src/glsl/ast_to_hir.cpp: "function `%s' in
> GLSL ES 3.00", name);
> src/glsl/ast_to_hir.cpp: "not allowed in GLSL
> ES 1.00");
> src/glsl/glsl_parser.yy:"(GLSL ES 1.00 or
> GLSL 1.20 required)",
> src/glsl/glsl_parser.yy:"%s qualifier
> requires GLSL 4.30 or "
> src/glsl/glsl_parser_extras.cpp: "GLSL 1.00
> ES should be selected using "
>
> The version strings reported also have the . in them:
>
> const char *
> glsl_compute_version_string(void *mem_ctx, bool is_es, unsigned version)
> {
>return ralloc_asprintf(mem_ctx, "GLSL%s %d.%02d", is_es ? " ES" : "",
> version / 100, version % 100);
> }
>
> IMHO it'd be better to standardize on one thing.
Yes I agree they should be all the same. I'll make a patch to do that
and see what others have to say about it.
>
> -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: binding point is a texture unit, which is a combined space
Looks good to me as well.
Reviewed-by: Timothy Arceri
Since you already looking around in there did you want to take a look
at this patch:
http://lists.freedesktop.org/archives/mesa-dev/2015-June/086925.html
I submitted it earlier this month but no one seems to keen to review
it.
On Tue, 2015-06-23 at 00:22 -0400, Ilia Mirkin wrote:
> This fixes compilation failures in Dota 2 Reborn where a texture unit
> binding point was used that was numerically higher than the max
> per stage.
>
> Signed-off-by: Ilia Mirkin
> Reviewed-by: Chris Forbes
> Tested-by: Nick Sarnie
> Cc: "10.5 10.6"
> ---
> src/glsl/ast_to_hir.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 259e01e..8cb46be 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2086,7 +2086,7 @@ validate_binding_qualifier(struct
> _mesa_glsl_parse_state *state,
> * with an array of size N, all elements of the array from
> binding
> * through binding + N - 1 must be within this range."
> */
> - unsigned limit = ctx->Const.Program[state
> ->stage].MaxTextureImageUnits;
> + unsigned limit = ctx->Const.MaxCombinedTextureImageUnits;
>
>if (max_index >= limit) {
> _mesa_glsl_error(loc, state, "layout(binding = %d) for %d
> samplers "
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: binding point is a texture unit, which is a combined space
This also probably explains why my max binding test for arrays of
arrays was failing on the Nvidia blob. Guess I should have looked into
that failure a little harder.
On Tue, 2015-06-23 at 00:22 -0400, Ilia Mirkin wrote:
> This fixes compilation failures in Dota 2 Reborn where a texture unit
> binding point was used that was numerically higher than the max
> per stage.
>
> Signed-off-by: Ilia Mirkin
> Reviewed-by: Chris Forbes
> Tested-by: Nick Sarnie
> Cc: "10.5 10.6"
> ---
> src/glsl/ast_to_hir.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 259e01e..8cb46be 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2086,7 +2086,7 @@ validate_binding_qualifier(struct
> _mesa_glsl_parse_state *state,
> * with an array of size N, all elements of the array from
> binding
> * through binding + N - 1 must be within this range."
> */
> - unsigned limit = ctx->Const.Program[state
> ->stage].MaxTextureImageUnits;
> + unsigned limit = ctx->Const.MaxCombinedTextureImageUnits;
>
>if (max_index >= limit) {
> _mesa_glsl_error(loc, state, "layout(binding = %d) for %d
> samplers "
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] glsl: use consistent version string format
---
src/glsl/glsl_parser_extras.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index 9a0c24e..02ddbbd 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -129,7 +129,7 @@ struct _mesa_glsl_parse_state {
bool check_explicit_attrib_stream_allowed(YYLTYPE *locp)
{
if (!this->has_explicit_attrib_stream()) {
- const char *const requirement = "GL_ARB_gpu_shader5 extension or GLSL
400";
+ const char *const requirement = "GL_ARB_gpu_shader5 extension or GLSL
4.00";
_mesa_glsl_error(locp, this, "explicit stream requires %s",
requirement);
@@ -144,8 +144,8 @@ struct _mesa_glsl_parse_state {
{
if (!this->has_explicit_attrib_location()) {
const char *const requirement = this->es_shader
-? "GLSL ES 300"
-: "GL_ARB_explicit_attrib_location extension or GLSL 330";
+? "GLSL ES 3.00"
+: "GL_ARB_explicit_attrib_location extension or GLSL 3.30";
_mesa_glsl_error(locp, this, "%s explicit location requires %s",
mode_string(var), requirement);
@@ -160,8 +160,8 @@ struct _mesa_glsl_parse_state {
{
if (!this->has_separate_shader_objects()) {
const char *const requirement = this->es_shader
-? "GL_EXT_separate_shader_objects extension or GLSL ES 310"
-: "GL_ARB_separate_shader_objects extension or GLSL 420";
+? "GL_EXT_separate_shader_objects extension or GLSL ES 3.10"
+: "GL_ARB_separate_shader_objects extension or GLSL 4.20";
_mesa_glsl_error(locp, this, "%s explicit location requires %s",
mode_string(var), requirement);
@@ -177,9 +177,9 @@ struct _mesa_glsl_parse_state {
if (!this->has_explicit_attrib_location() ||
!this->has_explicit_uniform_location()) {
const char *const requirement = this->es_shader
-? "GLSL ES 310"
+? "GLSL ES 3.10"
: "GL_ARB_explicit_uniform_location and either "
- "GL_ARB_explicit_attrib_location or GLSL 330.";
+ "GL_ARB_explicit_attrib_location or GLSL 3.30.";
_mesa_glsl_error(locp, this,
"uniform explicit location requires %s",
--
2.4.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] freedreno: use consistent version string format
---
src/gallium/drivers/freedreno/freedreno_screen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c
b/src/gallium/drivers/freedreno/freedreno_screen.c
index b3b5462..7330cd9 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -68,7 +68,7 @@ static const struct debug_named_value debug_options[] = {
{"fraghalf", FD_DBG_FRAGHALF, "Use half-precision in fragment
shader"},
{"nobin", FD_DBG_NOBIN, "Disable hw binning"},
{"optmsgs", FD_DBG_OPTMSGS,"Enable optimizer debug messages"},
- {"glsl120", FD_DBG_GLSL120,"Temporary flag to force GLSL 120
(rather than 130) on a3xx+"},
+ {"glsl120", FD_DBG_GLSL120,"Temporary flag to force GLSL 1.20
(rather than 1.30) on a3xx+"},
DEBUG_NAMED_VALUE_END
};
--
2.4.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] ARB_arrays_of_arrays GLSL ES
On Mon, 2015-06-22 at 19:04 +0300, Eero Tamminen wrote: > Hi, > > On 06/20/2015 03:32 PM, Timothy Arceri wrote: > > The restrictions in ES make the extension easier to implement so > > I thought I'd try get this stuff reviewed an committed before > > finishing > > up the full extension. > > The bits that I'm still working on for the desktop version are AoA > > inputs > > outputs, and interface blocks. > > > > The only thing I know is definatly missing in this series for ES is > > support for indirect indexing of samplers, but that didn't seem > > like > > something that should hold up the series. > > > > Once the SSBO series lands (with a patch that restricts unsized > > arrays) > > then all the AoA ES conformance tests will pass. > > > > There are already a bunch of piglit tests in git but I've just sent > > a > > series with all the patches still waiting review here: > > http://lists.freedesktop.org/archives/piglit/2015-June/016312.html > > > > I haven't made a patch marking this as done yet because currently > > the i965 backend takes a very long time trying to optimise some of > > the > > conformance tests. They still pass but they are taking 15-minutes+ > > just > > to compile so this really needs to be sorted out first. If someone > > with > > more knowledge in this area than me wants to take a look at this I > > would > > be greatful for being pointed in the right direction. > > Are there individual shaders which compilation take several minutes? Yes. The shaders are part of these tests: ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1 ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 Callgrinds reporting 65% in vec4_live_variables::compute_live_variables() with InteractionFunctionCalls1 > > Do you have any perf [1] or valgrind [2] tool output for compiling > the > slowest one? > > > - Eero > > [1] # perf record -a > ^C > # perf report -n > -> text output > [2] $ valgrind --tool=callgrind > $ kcachegrind > -> callgraphs, callee maps etc > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] ARB_arrays_of_arrays GLSL ES
On Wed, 2015-06-24 at 11:17 -0700, Jason Ekstrand wrote: > On Sat, Jun 20, 2015 at 5:32 AM, Timothy Arceri < > t_arc...@yahoo.com.au> wrote: > > Hi all, > > > > The restrictions in ES make the extension easier to implement so > > I thought I'd try get this stuff reviewed an committed before > > finishing > > up the full extension. > > The bits that I'm still working on for the desktop version are AoA > > inputs > > outputs, and interface blocks. > > > > The only thing I know is definatly missing in this series for ES is > > support for indirect indexing of samplers, but that didn't seem > > like > > something that should hold up the series. > > > > Once the SSBO series lands (with a patch that restricts unsized > > arrays) > > then all the AoA ES conformance tests will pass. > > > > There are already a bunch of piglit tests in git but I've just sent > > a > > series with all the patches still waiting review here: > > http://lists.freedesktop.org/archives/piglit/2015-June/016312.html > > > > I haven't made a patch marking this as done yet because currently > > the i965 backend takes a very long time trying to optimise some of > > the > > conformance tests. They still pass but they are taking 15-minutes+ > > just > > to compile so this really needs to be sorted out first. If someone > > with > > more knowledge in this area than me wants to take a look at this I > > would > > be greatful for being pointed in the right direction. > > Can you try it with this patch: > > http://lists.freedesktop.org/archives/mesa-dev/2015-June/086011.html Hi Jason, I tried that patch it didn't seem to make any difference, then I noticed its for fs. The slowdown is currently happening in vec4_live_variables. I've also noticed other large slowdowns with some piglit tests I've been working on this time in the register allocate code. Once I finish fixing up some bugs with my current patchset I'll start digging deeper, just thought since it's so noticeably slow it might be easy for someone with good knowledge of the backend to point out where I should start looking, or to say your doing this wrong. One thing the slow shaders have in common is the use of arrays of arrays in nested loops so maybe something funny is going on when they go through the optimisation paths, I haven't spent much time looking at any of these passes yet so its highly likely they don't work as expected for arrays of arrays. Thanks anyway, Tim > > > If useful the series is in the 'gles4' branch of the repo here: > > https://github.com/tarceri/Mesa_arrays_of_arrays.git > > > > Thanks, > > Tim > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] ARB_arrays_of_arrays GLSL ES
On Thu, 2015-06-25 at 10:07 -0700, Jason Ekstrand wrote: > On Thu, Jun 25, 2015 at 1:19 AM, Timothy Arceri < > t_arc...@yahoo.com.au> wrote: > > On Wed, 2015-06-24 at 11:17 -0700, Jason Ekstrand wrote: > > > On Sat, Jun 20, 2015 at 5:32 AM, Timothy Arceri < > > > t_arc...@yahoo.com.au> wrote: > > > > Hi all, > > > > > > > > The restrictions in ES make the extension easier to implement > > > > so > > > > I thought I'd try get this stuff reviewed an committed before > > > > finishing > > > > up the full extension. > > > > The bits that I'm still working on for the desktop version are > > > > AoA > > > > inputs > > > > outputs, and interface blocks. > > > > > > > > The only thing I know is definatly missing in this series for > > > > ES is > > > > support for indirect indexing of samplers, but that didn't seem > > > > like > > > > something that should hold up the series. > > > > > > > > Once the SSBO series lands (with a patch that restricts unsized > > > > arrays) > > > > then all the AoA ES conformance tests will pass. > > > > > > > > There are already a bunch of piglit tests in git but I've just > > > > sent > > > > a > > > > series with all the patches still waiting review here: > > > > http://lists.freedesktop.org/archives/piglit/2015 > > > > -June/016312.html > > > > > > > > I haven't made a patch marking this as done yet because > > > > currently > > > > the i965 backend takes a very long time trying to optimise some > > > > of > > > > the > > > > conformance tests. They still pass but they are taking 15 > > > > -minutes+ > > > > just > > > > to compile so this really needs to be sorted out first. If > > > > someone > > > > with > > > > more knowledge in this area than me wants to take a look at > > > > this I > > > > would > > > > be greatful for being pointed in the right direction. > > > > > > Can you try it with this patch: > > > > > > http://lists.freedesktop.org/archives/mesa-dev/2015 > > > -June/086011.html > > > > Hi Jason, > > > > I tried that patch it didn't seem to make any difference, then I > > noticed its for fs. The slowdown is currently happening in > > vec4_live_variables. I've also noticed other large slowdowns with > > some > > piglit tests I've been working on this time in the register > > allocate > > code. > > I just sent an equivalent patch for vec4: > > http://patchwork.freedesktop.org/patch/52801/ > > I would love to know if it helps something so that I can put a good > justification in the commit message beyond "the same as for FS". > --Jason > I ran the shortest of the two tests: ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1 With your patch: 3:31.40 elapsed Without your patch: 6:49.62 elapsed > > Once I finish fixing up some bugs with my current patchset I'll > > start > > digging deeper, just thought since it's so noticeably slow it might > > be > > easy for someone with good knowledge of the backend to point out > > where > > I should start looking, or to say your doing this wrong. > > > > One thing the slow shaders have in common is the use of arrays of > > arrays in nested loops so maybe something funny is going on when > > they > > go through the optimisation paths, I haven't spent much time > > looking at > > any of these passes yet so its highly likely they don't work as > > expected for arrays of arrays. > > > > Thanks anyway, > > Tim > > > > > > > > > > > If useful the series is in the 'gles4' branch of the repo here: > > > > https://github.com/tarceri/Mesa_arrays_of_arrays.git > > > > > > > > Thanks, > > > > Tim > > > > ___ > > > > mesa-dev mailing list > > > > mesa-dev@lists.freedesktop.org > > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] glsl: check for leading zeros in array index validation
Cc: Martin Peres Cc: Tapani Pälli --- src/glsl/linker.cpp | 4 1 file changed, 4 insertions(+) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 71a45e8..d8f1689 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -462,6 +462,10 @@ parse_program_resource_name(const GLchar *name, if (array_index < 0) return -1; + /* Check for leading zero */ + if (name[i] == '0' && name[i+1] != ']') + return -1; + *out_base_name_end = name + (i - 1); return array_index; } -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] mesa: remove duplicate array index validation and extra uniform location lookup
This change removes multiple functions designed to validate an array
subscript and replaces them with a call to a single function.
The change also means that validation is now only done once and the index
is retrived at the same time, as a result the getUniformLocation code can
be merged saving an extra hash table lookup (and yet another validation call).
Cc: Martin Peres
Cc: Tapani Pälli
---
This clean-up was done to allow AoA program interface query support to be
implemented more easily. When applied to my latest AoA work all but one
AoA program interface query subtest for the CTS now passes.
No Piglit regressions.
I also ran the following tests in the CTS without regression:
- ES31-CTS.program_interface_query*
- ES3-CTS.gtf.GL3Tests.explicit_attrib_location.explicit_attrib_location_room
- ES3-CTS.gtf.GL3Tests.draw_instanced.draw_instanced_max_vertex_attribs
src/mesa/main/program_resource.c | 66 ++---
src/mesa/main/shader_query.cpp | 206 ---
src/mesa/main/shaderapi.h| 7 +-
src/mesa/main/uniform_query.cpp | 75 --
src/mesa/main/uniforms.c | 7 +-
src/mesa/main/uniforms.h | 4 -
6 files changed, 100 insertions(+), 265 deletions(-)
diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index d857b84..0a306b8 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -173,32 +173,12 @@ is_xfb_marker(const char *str)
return false;
}
-/**
- * Checks if given name index is legal for GetProgramResourceIndex,
- * check is written to be compatible with GL_ARB_array_of_arrays.
- */
-static bool
-valid_program_resource_index_name(const GLchar *name)
-{
- const char *array = strstr(name, "[");
- const char *close = strrchr(name, ']');
-
- /* Not array, no need for the check. */
- if (!array)
- return true;
-
- /* Last array index has to be zero. */
- if (!close || *--close != '0')
- return false;
-
- return true;
-}
-
GLuint GLAPIENTRY
_mesa_GetProgramResourceIndex(GLuint program, GLenum programInterface,
const GLchar *name)
{
GET_CURRENT_CONTEXT(ctx);
+ unsigned array_index;
struct gl_program_resource *res;
struct gl_shader_program *shProg =
_mesa_lookup_shader_program_err(ctx, program,
@@ -221,12 +201,9 @@ _mesa_GetProgramResourceIndex(GLuint program, GLenum
programInterface,
case GL_PROGRAM_OUTPUT:
case GL_UNIFORM:
case GL_TRANSFORM_FEEDBACK_VARYING:
- /* Validate name syntax for array variables */
- if (!valid_program_resource_index_name(name))
- return GL_INVALID_INDEX;
- /* fall-through */
case GL_UNIFORM_BLOCK:
- res = _mesa_program_resource_find_name(shProg, programInterface, name);
+ res = _mesa_program_resource_find_name(shProg, programInterface, name,
+ &array_index, true);
if (!res)
return GL_INVALID_INDEX;
@@ -300,36 +277,6 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum
programInterface,
propCount, props, bufSize, length, params);
}
-/**
- * Function verifies syntax of given name for GetProgramResourceLocation
- * and GetProgramResourceLocationIndex for the following cases:
- *
- * "array element portion of a string passed to GetProgramResourceLocation
- * or GetProgramResourceLocationIndex must not have, a "+" sign, extra
- * leading zeroes, or whitespace".
- *
- * Check is written to be compatible with GL_ARB_array_of_arrays.
- */
-static bool
-invalid_array_element_syntax(const GLchar *name)
-{
- char *first = strchr(name, '[');
- char *last = strrchr(name, '[');
-
- if (!first)
- return false;
-
- /* No '+' or ' ' allowed anywhere. */
- if (strchr(first, '+') || strchr(first, ' '))
- return true;
-
- /* Check that last array index is 0. */
- if (last[1] == '0' && last[2] != ']')
- return true;
-
- return false;
-}
-
static struct gl_shader_program *
lookup_linked_program(GLuint program, const char *caller)
{
@@ -356,7 +303,7 @@ _mesa_GetProgramResourceLocation(GLuint program, GLenum
programInterface,
struct gl_shader_program *shProg =
lookup_linked_program(program, "glGetProgramResourceLocation");
- if (!shProg || !name || invalid_array_element_syntax(name))
+ if (!shProg || !name)
return -1;
/* Validate programInterface. */
@@ -383,7 +330,8 @@ _mesa_GetProgramResourceLocation(GLuint program, GLenum
programInterface,
_mesa_lookup_enum_by_nr(programInterface), name);
}
- return _mesa_program_resource_location(shProg, programInterface, name);
+ return _mesa_program_resource_location(shProg, programInterface, name,
+ true);
}
/**
@@ -397,7 +345,7 @@ _mesa_GetProgramResourceLocationIndex(GLuint program,
GLenum programInterface,
struct gl_shader_program *shProg
Re: [Mesa-dev] [PATCH 2/2] mesa: remove duplicate array index validation and extra uniform location lookup
On Fri, 2015-07-03 at 15:08 +0300, Martin Peres wrote:
>
> On 03/07/15 13:55, Timothy Arceri wrote:
> > This change removes multiple functions designed to validate an array
> > subscript and replaces them with a call to a single function.
> >
> > The change also means that validation is now only done once and the index
> > is retrived at the same time, as a result the getUniformLocation code can
> > be merged saving an extra hash table lookup (and yet another validation
> > call).
> >
> > Cc: Martin Peres
> > Cc: Tapani Pälli
> > ---
> > This clean-up was done to allow AoA program interface query support to
> > be
> > implemented more easily. When applied to my latest AoA work all but one
> > AoA program interface query subtest for the CTS now passes.
> >
> > No Piglit regressions.
> >
> > I also ran the following tests in the CTS without regression:
> >
> > - ES31-CTS.program_interface_query*
> > - ES3
> > -CTS.gtf.GL3Tests.explicit_attrib_location.explicit_attrib_location_room
> > - ES3-CTS.gtf.GL3Tests.draw_instanced.draw_instanced_max_vertex_attribs
> >
> > src/mesa/main/program_resource.c | 66 ++---
> > src/mesa/main/shader_query.cpp | 206 -
> > --
> > src/mesa/main/shaderapi.h| 7 +-
> > src/mesa/main/uniform_query.cpp | 75 --
> > src/mesa/main/uniforms.c | 7 +-
> > src/mesa/main/uniforms.h | 4 -
> > 6 files changed, 100 insertions(+), 265 deletions(-)
> >
> > diff --git a/src/mesa/main/program_resource.c
> > b/src/mesa/main/program_resource.c
> > index d857b84..0a306b8 100644
> > --- a/src/mesa/main/program_resource.c
> > +++ b/src/mesa/main/program_resource.c
> > @@ -173,32 +173,12 @@ is_xfb_marker(const char *str)
> > return false;
> > }
> >
> > -/**
> > - * Checks if given name index is legal for GetProgramResourceIndex,
> > - * check is written to be compatible with GL_ARB_array_of_arrays.
> > - */
> > -static bool
> > -valid_program_resource_index_name(const GLchar *name)
> > -{
> > - const char *array = strstr(name, "[");
> > - const char *close = strrchr(name, ']');
> > -
> > - /* Not array, no need for the check. */
> > - if (!array)
> > - return true;
> > -
> > - /* Last array index has to be zero. */
> > - if (!close || *--close != '0')
> > - return false;
> > -
> > - return true;
> > -}
> > -
> > GLuint GLAPIENTRY
> > _mesa_GetProgramResourceIndex(GLuint program, GLenum programInterface,
> > const GLchar *name)
> > {
> > GET_CURRENT_CONTEXT(ctx);
> > + unsigned array_index;
> > struct gl_program_resource *res;
> > struct gl_shader_program *shProg =
> > _mesa_lookup_shader_program_err(ctx, program,
> > @@ -221,12 +201,9 @@ _mesa_GetProgramResourceIndex(GLuint program, GLenum
> > programInterface,
> > case GL_PROGRAM_OUTPUT:
> > case GL_UNIFORM:
> > case GL_TRANSFORM_FEEDBACK_VARYING:
> > - /* Validate name syntax for array variables */
> > - if (!valid_program_resource_index_name(name))
> > - return GL_INVALID_INDEX;
> > - /* fall-through */
> > case GL_UNIFORM_BLOCK:
> > - res = _mesa_program_resource_find_name(shProg, programInterface,
> > name);
> > + res = _mesa_program_resource_find_name(shProg, programInterface,
> > name,
> > + &array_index, true);
> > if (!res)
> >return GL_INVALID_INDEX;
> >
> > @@ -300,36 +277,6 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum
> > programInterface,
> > propCount, props, bufSize, length,
> > params);
> > }
> >
> > -/**
> > - * Function verifies syntax of given name for GetProgramResourceLocation
> > - * and GetProgramResourceLocationIndex for the following cases:
> > - *
> > - * "array element portion of a string passed to
> > GetProgramResourceLocation
> > - * or GetProgramResourceLocationIndex must not have, a "+" sign, extra
> > - * leading zeroes, or whitespace".
> > - *
> > - * Check is written to be compatible with GL_ARB_array_of_arrays.
> > - */
> > -static bool
> > -invalid_array_element_syntax(const GLchar *name)
> > -{
&g
[Mesa-dev] [PATCH V2 4/4] mesa: remove now unused _mesa_get_uniform_location
Cc: Tapani Pälli
---
src/mesa/main/uniform_query.cpp | 75 -
src/mesa/main/uniforms.h| 4 ---
2 files changed, 79 deletions(-)
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index cab5083..680ba16 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -978,81 +978,6 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct
gl_shader_program *shProg,
}
-/**
- * Called via glGetUniformLocation().
- *
- * Returns the uniform index into UniformStorage (also the
- * glGetActiveUniformsiv uniform index), and stores the referenced
- * array offset in *offset, or GL_INVALID_INDEX (-1).
- */
-extern "C" unsigned
-_mesa_get_uniform_location(struct gl_shader_program *shProg,
- const GLchar *name,
- unsigned *out_offset)
-{
- /* Page 80 (page 94 of the PDF) of the OpenGL 2.1 spec says:
-*
-* "The first element of a uniform array is identified using the
-* name of the uniform array appended with "[0]". Except if the last
-* part of the string name indicates a uniform array, then the
-* location of the first element of that array can be retrieved by
-* either using the name of the uniform array, or the name of the
-* uniform array appended with "[0]"."
-*
-* Note: since uniform names are not allowed to use whitespace, and array
-* indices within uniform names are not allowed to use "+", "-", or leading
-* zeros, it follows that each uniform has a unique name up to the possible
-* ambiguity with "[0]" noted above. Therefore we don't need to worry
-* about mal-formed inputs--they will properly fail when we try to look up
-* the uniform name in shProg->UniformHash.
-*/
-
- const GLchar *base_name_end;
- long offset = parse_program_resource_name(name, &base_name_end);
- bool array_lookup = offset >= 0;
- char *name_copy;
-
- if (array_lookup) {
- name_copy = (char *) malloc(base_name_end - name + 1);
- memcpy(name_copy, name, base_name_end - name);
- name_copy[base_name_end - name] = '\0';
- } else {
- name_copy = (char *) name;
- offset = 0;
- }
-
- unsigned location = 0;
- const bool found = shProg->UniformHash->get(location, name_copy);
-
- assert(!found
- || strcmp(name_copy, shProg->UniformStorage[location].name) == 0);
-
- /* Free the temporary buffer *before* possibly returning an error.
-*/
- if (name_copy != name)
- free(name_copy);
-
- if (!found)
- return GL_INVALID_INDEX;
-
- /* If the uniform is built-in, fail. */
- if (shProg->UniformStorage[location].builtin)
- return GL_INVALID_INDEX;
-
- /* If the uniform is an array, fail if the index is out of bounds.
-* (A negative index is caught above.) This also fails if the uniform
-* is not an array, but the user is trying to index it, because
-* array_elements is zero and offset >= 0.
-*/
- if (array_lookup
- && offset >= (long) shProg->UniformStorage[location].array_elements) {
- return GL_INVALID_INDEX;
- }
-
- *out_offset = offset;
- return location;
-}
-
extern "C" bool
_mesa_sampler_uniforms_are_valid(const struct gl_shader_program *shProg,
char *errMsg, size_t errMsgLength)
diff --git a/src/mesa/main/uniforms.h b/src/mesa/main/uniforms.h
index bd7b05e..e62eaa5 100644
--- a/src/mesa/main/uniforms.h
+++ b/src/mesa/main/uniforms.h
@@ -343,10 +343,6 @@ void GLAPIENTRY
_mesa_ProgramUniformMatrix4x3dv(GLuint program, GLint location, GLsizei count,
GLboolean transpose, const GLdouble *value);
-unsigned
-_mesa_get_uniform_location(struct gl_shader_program *shProg,
- const GLchar *name, unsigned *offset);
-
void
_mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shader_program,
GLint location, GLsizei count,
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH V2 3/4] mesa: remove overlapping array index validation and extra uniform location lookup
This change removes multiple functions designed to validate an array
subscript and replaces them with a call to a single function.
The change also means that validation is now only done once and the index
is retrived at the same time, as a result the getUniformLocation code can
be simplified saving an extra hash table lookup (and yet another
validation call).
V2: Fix bounds checks for program input/output, split unrelated comment fix
and _mesa_get_uniform_location() removal into their own patch.
Cc: Tapani Pälli
---
src/mesa/main/program_resource.c | 66 ++---
src/mesa/main/shader_query.cpp | 204 ---
src/mesa/main/shaderapi.h| 7 +-
src/mesa/main/uniforms.c | 7 +-
4 files changed, 100 insertions(+), 184 deletions(-)
diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index d857b84..0a306b8 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -173,32 +173,12 @@ is_xfb_marker(const char *str)
return false;
}
-/**
- * Checks if given name index is legal for GetProgramResourceIndex,
- * check is written to be compatible with GL_ARB_array_of_arrays.
- */
-static bool
-valid_program_resource_index_name(const GLchar *name)
-{
- const char *array = strstr(name, "[");
- const char *close = strrchr(name, ']');
-
- /* Not array, no need for the check. */
- if (!array)
- return true;
-
- /* Last array index has to be zero. */
- if (!close || *--close != '0')
- return false;
-
- return true;
-}
-
GLuint GLAPIENTRY
_mesa_GetProgramResourceIndex(GLuint program, GLenum programInterface,
const GLchar *name)
{
GET_CURRENT_CONTEXT(ctx);
+ unsigned array_index;
struct gl_program_resource *res;
struct gl_shader_program *shProg =
_mesa_lookup_shader_program_err(ctx, program,
@@ -221,12 +201,9 @@ _mesa_GetProgramResourceIndex(GLuint program, GLenum
programInterface,
case GL_PROGRAM_OUTPUT:
case GL_UNIFORM:
case GL_TRANSFORM_FEEDBACK_VARYING:
- /* Validate name syntax for array variables */
- if (!valid_program_resource_index_name(name))
- return GL_INVALID_INDEX;
- /* fall-through */
case GL_UNIFORM_BLOCK:
- res = _mesa_program_resource_find_name(shProg, programInterface, name);
+ res = _mesa_program_resource_find_name(shProg, programInterface, name,
+ &array_index, true);
if (!res)
return GL_INVALID_INDEX;
@@ -300,36 +277,6 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum
programInterface,
propCount, props, bufSize, length, params);
}
-/**
- * Function verifies syntax of given name for GetProgramResourceLocation
- * and GetProgramResourceLocationIndex for the following cases:
- *
- * "array element portion of a string passed to GetProgramResourceLocation
- * or GetProgramResourceLocationIndex must not have, a "+" sign, extra
- * leading zeroes, or whitespace".
- *
- * Check is written to be compatible with GL_ARB_array_of_arrays.
- */
-static bool
-invalid_array_element_syntax(const GLchar *name)
-{
- char *first = strchr(name, '[');
- char *last = strrchr(name, '[');
-
- if (!first)
- return false;
-
- /* No '+' or ' ' allowed anywhere. */
- if (strchr(first, '+') || strchr(first, ' '))
- return true;
-
- /* Check that last array index is 0. */
- if (last[1] == '0' && last[2] != ']')
- return true;
-
- return false;
-}
-
static struct gl_shader_program *
lookup_linked_program(GLuint program, const char *caller)
{
@@ -356,7 +303,7 @@ _mesa_GetProgramResourceLocation(GLuint program, GLenum
programInterface,
struct gl_shader_program *shProg =
lookup_linked_program(program, "glGetProgramResourceLocation");
- if (!shProg || !name || invalid_array_element_syntax(name))
+ if (!shProg || !name)
return -1;
/* Validate programInterface. */
@@ -383,7 +330,8 @@ _mesa_GetProgramResourceLocation(GLuint program, GLenum
programInterface,
_mesa_lookup_enum_by_nr(programInterface), name);
}
- return _mesa_program_resource_location(shProg, programInterface, name);
+ return _mesa_program_resource_location(shProg, programInterface, name,
+ true);
}
/**
@@ -397,7 +345,7 @@ _mesa_GetProgramResourceLocationIndex(GLuint program,
GLenum programInterface,
struct gl_shader_program *shProg =
lookup_linked_program(program, "glGetProgramResourceLocationIndex");
- if (!shProg || !name || invalid_array_element_syntax(name))
+ if (!shProg || !name)
return -1;
/* From the GL_ARB_program_interface_query spec:
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 0473c2e..3b08ea1 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -44,7 +44,8 @@ extern "C" {
st
[Mesa-dev] [PATCH V2 2/4] mesa: fix incorrect comment
Cc: Tapani Pälli --- src/mesa/main/shader_query.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index a6246a3..0473c2e 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -787,8 +787,6 @@ program_resource_location(struct gl_shader_program *shProg, /** * Function implements following location queries: - *glGetAttribLocation - *glGetFragDataLocation *glGetUniformLocation */ GLint -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH V2 1/4] glsl: check for leading zeros in array index validation
Cc: Tapani Pälli --- src/glsl/linker.cpp | 4 1 file changed, 4 insertions(+) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 71a45e8..d8f1689 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -462,6 +462,10 @@ parse_program_resource_name(const GLchar *name, if (array_index < 0) return -1; + /* Check for leading zero */ + if (name[i] == '0' && name[i+1] != ']') + return -1; + *out_base_name_end = name + (i - 1); return array_index; } -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: set stage flag for structs and arrays in resource list
This fixes: ES31-CTS.program_interface_query.uniform-types
Cc: Tapani Pälli
---
Note: This only fixes the remaining CTS subtests for uniform-types the
other fixes are part of my clean-up series:
http://lists.freedesktop.org/archives/mesa-dev/2015-July/088122.html
src/glsl/linker.cpp | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 0d4bc15..6a69c15 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -2648,9 +2648,19 @@ build_stageref(struct gl_shader_program *shProg, const
char *name)
*/
foreach_in_list(ir_instruction, node, sh->ir) {
ir_variable *var = node->as_variable();
- if (var && strcmp(var->name, name) == 0) {
-stages |= (1 << i);
-break;
+ if (var) {
+unsigned baselen = strlen(var->name);
+if (strncmp(var->name, name, baselen) == 0) {
+ /* Check for exact name matches but also check for arrays and
+* structs.
+*/
+ if (name[baselen] == '\0' ||
+ name[baselen] == '[' ||
+ name[baselen] == '.') {
+ stages |= (1 << i);
+ break;
+ }
+}
}
}
}
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2 1/4] glsl: check for leading zeros in array index validation
I didn't notice earlier but it seems this series also fixes some CTS subtests for example some test are fixed in ES31-CTS.program_interface_query.uniform -types. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH V2] mesa: fix active sampler conflict validation
The type stored in gl_uniform_storage is the type of a single array
element not the array type so size was always 1. Use the number of array
elements stored in the gl_uniform_storage instead.
V2: Dont validate sampler units pointing to 0
---
Fixes new piglit test:
http://lists.freedesktop.org/archives/piglit/2015-July/016452.html
src/mesa/main/uniform_query.cpp | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index 680ba16..036530e 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -1026,18 +1026,23 @@ _mesa_sampler_uniforms_pipeline_are_valid(struct
gl_pipeline_object *pipeline)
for (unsigned i = 0; i < shProg[idx]->NumUniformStorage; i++) {
const struct gl_uniform_storage *const storage =
&shProg[idx]->UniformStorage[i];
- const glsl_type *const t = (storage->type->is_array())
-? storage->type->fields.array : storage->type;
- if (!t->is_sampler())
+ if (!storage->type->is_sampler())
continue;
active_samplers++;
- const unsigned count = MAX2(1, storage->type->array_size());
+ const unsigned count = MAX2(1, storage->array_elements);
for (unsigned j = 0; j < count; j++) {
const unsigned unit = storage->storage[j].i;
+/* FIXME: Samplers are initialized to 0 and Mesa doesn't do a
+ * great job of eliminating unused uniforms currently so for now
+ * don't throw an error if two sampler types both point to 0.
+ */
+if (unit == 0)
+ continue;
+
/* The types of the samplers associated with a particular texture
* unit must be an exact match. Page 74 (page 89 of the PDF) of
* the OpenGL 3.3 core spec says:
@@ -1047,13 +1052,14 @@ _mesa_sampler_uniforms_pipeline_are_valid(struct
gl_pipeline_object *pipeline)
* program object."
*/
if (unit_types[unit] == NULL) {
- unit_types[unit] = t;
-} else if (unit_types[unit] != t) {
+ unit_types[unit] = storage->type;
+} else if (unit_types[unit] != storage->type) {
pipeline->InfoLog =
ralloc_asprintf(pipeline,
"Texture unit %d is accessed both as %s "
"and %s",
- unit, unit_types[unit]->name, t->name);
+ unit, unit_types[unit]->name,
+ storage->type->name);
return false;
}
}
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: use implementation specified MAX_VERTEX_ATTRIBS rather than hardcoded value
---
src/glsl/linker.cpp | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 6a69c15..2f5a36f 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -3084,12 +3084,7 @@ link_shaders(struct gl_context *ctx, struct
gl_shader_program *prog)
}
}
- /* FINISHME: The value of the max_attribute_index parameter is
-* FINISHME: implementation dependent based on the value of
-* FINISHME: GL_MAX_VERTEX_ATTRIBS. GL_MAX_VERTEX_ATTRIBS must be
-* FINISHME: at least 16, so hardcode 16 for now.
-*/
- if (!assign_attribute_or_color_locations(prog, MESA_SHADER_VERTEX, 16)) {
+ if (!assign_attribute_or_color_locations(prog, MESA_SHADER_VERTEX,
ctx->Const.Program[MESA_SHADER_VERTEX].MaxAttribs)) {
goto done;
}
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: use the fast hash table for ir_validate
---
Some of the AoA tests currently get stuck in an optimisation
loop for a long time and this was taking a large amount of time.
Its a symptom not a cause but I don't see any issue with using the faster
version.
src/glsl/ir_validate.cpp | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
index cfe0df3..f3752bd 100644
--- a/src/glsl/ir_validate.cpp
+++ b/src/glsl/ir_validate.cpp
@@ -35,7 +35,7 @@
#include "ir.h"
#include "ir_hierarchical_visitor.h"
-#include "program/hash_table.h"
+#include "util/hash_table.h"
#include "glsl_types.h"
namespace {
@@ -44,8 +44,8 @@ class ir_validate : public ir_hierarchical_visitor {
public:
ir_validate()
{
- this->ht = hash_table_ctor(0, hash_table_pointer_hash,
-hash_table_pointer_compare);
+ this->ht = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
+ _mesa_key_pointer_equal);
this->current_function = NULL;
@@ -55,7 +55,7 @@ public:
~ir_validate()
{
- hash_table_dtor(this->ht);
+ _mesa_hash_table_destroy(this->ht, NULL);
}
virtual ir_visitor_status visit(ir_variable *v);
@@ -94,7 +94,7 @@ ir_validate::visit(ir_dereference_variable *ir)
abort();
}
- if (hash_table_find(ht, ir->var) == NULL) {
+ if (_mesa_hash_table_search(ht, ir->var) == NULL) {
printf("ir_dereference_variable @ %p specifies undeclared variable "
"`%s' @ %p\n",
(void *) ir, ir->var->name, (void *) ir->var);
@@ -730,8 +730,7 @@ ir_validate::visit(ir_variable *ir)
if (ir->name && ir->is_name_ralloced())
assert(ralloc_parent(ir->name) == ir);
- hash_table_insert(ht, ir, ir);
-
+ _mesa_hash_table_insert(ht, ir, ir);
/* If a variable is an array, verify that the maximum array index is in
* bounds. There was once an error in AST-to-HIR conversion that set this
@@ -887,13 +886,13 @@ ir_validate::validate_ir(ir_instruction *ir, void *data)
{
struct hash_table *ht = (struct hash_table *) data;
- if (hash_table_find(ht, ir)) {
+ if (_mesa_hash_table_search(ht, ir)) {
printf("Instruction node present twice in ir tree:\n");
ir->print();
printf("\n");
abort();
}
- hash_table_insert(ht, ir, ir);
+ _mesa_hash_table_insert(ht, ir, ir);
}
void
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: use set rather than old hash table for ir_validate
This implementation should be faster and there was no
need to store a data field.
---
src/glsl/ir_validate.cpp | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
index cfe0df3..684bef2 100644
--- a/src/glsl/ir_validate.cpp
+++ b/src/glsl/ir_validate.cpp
@@ -35,7 +35,8 @@
#include "ir.h"
#include "ir_hierarchical_visitor.h"
-#include "program/hash_table.h"
+#include "util/hash_table.h"
+#include "util/set.h"
#include "glsl_types.h"
namespace {
@@ -44,18 +45,18 @@ class ir_validate : public ir_hierarchical_visitor {
public:
ir_validate()
{
- this->ht = hash_table_ctor(0, hash_table_pointer_hash,
-hash_table_pointer_compare);
+ this->ir_set = _mesa_set_create(NULL, _mesa_hash_pointer,
+ _mesa_key_pointer_equal);
this->current_function = NULL;
this->callback_enter = ir_validate::validate_ir;
- this->data_enter = ht;
+ this->data_enter = ir_set;
}
~ir_validate()
{
- hash_table_dtor(this->ht);
+ _mesa_set_destroy(this->ir_set, NULL);
}
virtual ir_visitor_status visit(ir_variable *v);
@@ -80,7 +81,7 @@ public:
ir_function *current_function;
- struct hash_table *ht;
+ struct set *ir_set;
};
} /* anonymous namespace */
@@ -94,7 +95,7 @@ ir_validate::visit(ir_dereference_variable *ir)
abort();
}
- if (hash_table_find(ht, ir->var) == NULL) {
+ if (_mesa_set_search(ir_set, ir->var) == NULL) {
printf("ir_dereference_variable @ %p specifies undeclared variable "
"`%s' @ %p\n",
(void *) ir, ir->var->name, (void *) ir->var);
@@ -730,8 +731,7 @@ ir_validate::visit(ir_variable *ir)
if (ir->name && ir->is_name_ralloced())
assert(ralloc_parent(ir->name) == ir);
- hash_table_insert(ht, ir, ir);
-
+ _mesa_set_add(ir_set, ir);
/* If a variable is an array, verify that the maximum array index is in
* bounds. There was once an error in AST-to-HIR conversion that set this
@@ -885,15 +885,15 @@ dump_ir:
void
ir_validate::validate_ir(ir_instruction *ir, void *data)
{
- struct hash_table *ht = (struct hash_table *) data;
+ struct set *ir_set = (struct set *) data;
- if (hash_table_find(ht, ir)) {
+ if (_mesa_set_search(ir_set, ir)) {
printf("Instruction node present twice in ir tree:\n");
ir->print();
printf("\n");
abort();
}
- hash_table_insert(ht, ir, ir);
+ _mesa_set_add(ir_set, ir);
}
void
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: use the fast hash table for ir_validate
On Thu, 2015-07-09 at 17:50 -0700, Eric Anholt wrote:
> Timothy Arceri writes:
>
> > ---
> >
> > Some of the AoA tests currently get stuck in an optimisation
> > loop for a long time and this was taking a large amount of time.
> > Its a symptom not a cause but I don't see any issue with using the faster
> > version.
> >
> > src/glsl/ir_validate.cpp | 17 -
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
> > index cfe0df3..f3752bd 100644
> > --- a/src/glsl/ir_validate.cpp
> > +++ b/src/glsl/ir_validate.cpp
> > @@ -35,7 +35,7 @@
> >
> > #include "ir.h"
> > #include "ir_hierarchical_visitor.h"
> > -#include "program/hash_table.h"
> > +#include "util/hash_table.h"
> > #include "glsl_types.h"
> >
> > namespace {
> > @@ -44,8 +44,8 @@ class ir_validate : public ir_hierarchical_visitor {
> > public:
> > ir_validate()
> > {
> > - this->ht = hash_table_ctor(0, hash_table_pointer_hash,
> > -hash_table_pointer_compare);
> > + this->ht = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
> > + _mesa_key_pointer_equal);
> >
> >this->current_function = NULL;
> >
> > @@ -55,7 +55,7 @@ public:
> >
> > ~ir_validate()
> > {
> > - hash_table_dtor(this->ht);
> > + _mesa_hash_table_destroy(this->ht, NULL);
> > }
> >
> > virtual ir_visitor_status visit(ir_variable *v);
> > @@ -94,7 +94,7 @@ ir_validate::visit(ir_dereference_variable *ir)
> >abort();
> > }
> >
> > - if (hash_table_find(ht, ir->var) == NULL) {
> > + if (_mesa_hash_table_search(ht, ir->var) == NULL) {
> >printf("ir_dereference_variable @ %p specifies undeclared variable
> > "
> > "`%s' @ %p\n",
> > (void *) ir, ir->var->name, (void *) ir->var);
> > @@ -730,8 +730,7 @@ ir_validate::visit(ir_variable *ir)
> > if (ir->name && ir->is_name_ralloced())
> >assert(ralloc_parent(ir->name) == ir);
> >
> > - hash_table_insert(ht, ir, ir);
> > -
> > + _mesa_hash_table_insert(ht, ir, ir);
> >
> > /* If a variable is an array, verify that the maximum array index is
> > in
> > * bounds. There was once an error in AST-to-HIR conversion that set
> > this
> > @@ -887,13 +886,13 @@ ir_validate::validate_ir(ir_instruction *ir, void
> > *data)
> > {
> > struct hash_table *ht = (struct hash_table *) data;
> >
> > - if (hash_table_find(ht, ir)) {
> > + if (_mesa_hash_table_search(ht, ir)) {
> >printf("Instruction node present twice in ir tree:\n");
> >ir->print();
> >printf("\n");
> >abort();
> > }
> > - hash_table_insert(ht, ir, ir);
> > + _mesa_hash_table_insert(ht, ir, ir);
> > }
>
> It looks like you should use a set instead of a hash table, since you
> don't need a separate data field.
Right thanks, I've sent a new patch.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: use set rather than old hash table for ir_validate
On Fri, 2015-07-10 at 19:07 +1200, Chris Forbes wrote:
> Perf data?
I can create some if you like, but wasn't program/hash_table.c meant to die
along time ago [1] anyway.
[1] http://lists.freedesktop.org/archives/mesa-dev/2013-December/050524.html
>
> On Fri, Jul 10, 2015 at 6:41 PM, Timothy Arceri
> wrote:
> > This implementation should be faster and there was no
> > need to store a data field.
> > ---
> > src/glsl/ir_validate.cpp | 24
> > 1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
> > index cfe0df3..684bef2 100644
> > --- a/src/glsl/ir_validate.cpp
> > +++ b/src/glsl/ir_validate.cpp
> > @@ -35,7 +35,8 @@
> >
> > #include "ir.h"
> > #include "ir_hierarchical_visitor.h"
> > -#include "program/hash_table.h"
> > +#include "util/hash_table.h"
> > +#include "util/set.h"
> > #include "glsl_types.h"
> >
> > namespace {
> > @@ -44,18 +45,18 @@ class ir_validate : public ir_hierarchical_visitor {
> > public:
> > ir_validate()
> > {
> > - this->ht = hash_table_ctor(0, hash_table_pointer_hash,
> > -hash_table_pointer_compare);
> > + this->ir_set = _mesa_set_create(NULL, _mesa_hash_pointer,
> > + _mesa_key_pointer_equal);
> >
> >this->current_function = NULL;
> >
> >this->callback_enter = ir_validate::validate_ir;
> > - this->data_enter = ht;
> > + this->data_enter = ir_set;
> > }
> >
> > ~ir_validate()
> > {
> > - hash_table_dtor(this->ht);
> > + _mesa_set_destroy(this->ir_set, NULL);
> > }
> >
> > virtual ir_visitor_status visit(ir_variable *v);
> > @@ -80,7 +81,7 @@ public:
> >
> > ir_function *current_function;
> >
> > - struct hash_table *ht;
> > + struct set *ir_set;
> > };
> >
> > } /* anonymous namespace */
> > @@ -94,7 +95,7 @@ ir_validate::visit(ir_dereference_variable *ir)
> >abort();
> > }
> >
> > - if (hash_table_find(ht, ir->var) == NULL) {
> > + if (_mesa_set_search(ir_set, ir->var) == NULL) {
> >printf("ir_dereference_variable @ %p specifies undeclared variable
> > "
> > "`%s' @ %p\n",
> > (void *) ir, ir->var->name, (void *) ir->var);
> > @@ -730,8 +731,7 @@ ir_validate::visit(ir_variable *ir)
> > if (ir->name && ir->is_name_ralloced())
> >assert(ralloc_parent(ir->name) == ir);
> >
> > - hash_table_insert(ht, ir, ir);
> > -
> > + _mesa_set_add(ir_set, ir);
> >
> > /* If a variable is an array, verify that the maximum array index is
> > in
> > * bounds. There was once an error in AST-to-HIR conversion that set
> > this
> > @@ -885,15 +885,15 @@ dump_ir:
> > void
> > ir_validate::validate_ir(ir_instruction *ir, void *data)
> > {
> > - struct hash_table *ht = (struct hash_table *) data;
> > + struct set *ir_set = (struct set *) data;
> >
> > - if (hash_table_find(ht, ir)) {
> > + if (_mesa_set_search(ir_set, ir)) {
> >printf("Instruction node present twice in ir tree:\n");
> >ir->print();
> >printf("\n");
> >abort();
> > }
> > - hash_table_insert(ht, ir, ir);
> > + _mesa_set_add(ir_set, ir);
> > }
> >
> > void
> > --
> > 2.4.3
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: free interface_types and replace old hash_table uses
The util/hash_table was intended to be a fast hash table
replacement for the program/hash_table see 35fd61bd99c1 and 72e55bb6888ff.
This replaces some more uses of the old hash table and also
destroys the interface_types hash table when _mesa_glsl_release_types()
is called which wasn't previously being done.
---
Was looking at the remaining program/hash_table uses and noticed that
interface_types wasnt being freed so thought I'd fix that and replace the
hash while I was there.
No measurable compile time changes to the public shader-db
src/glsl/glsl_types.cpp | 85 ++---
src/glsl/glsl_types.h | 2 +-
2 files changed, 46 insertions(+), 41 deletions(-)
diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index 281ff51..255bd69 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -25,7 +25,7 @@
#include "main/core.h" /* for Elements, MAX2 */
#include "glsl_parser_extras.h"
#include "glsl_types.h"
-#include "program/hash_table.h"
+#include "util/hash_table.h"
mtx_t glsl_type::mutex = _MTX_INITIALIZER_NP;
@@ -329,14 +329,19 @@ _mesa_glsl_release_types(void)
* necessary.
*/
if (glsl_type::array_types != NULL) {
- hash_table_dtor(glsl_type::array_types);
+ _mesa_hash_table_destroy(glsl_type::array_types, NULL);
glsl_type::array_types = NULL;
}
if (glsl_type::record_types != NULL) {
- hash_table_dtor(glsl_type::record_types);
+ _mesa_hash_table_destroy(glsl_type::record_types, NULL);
glsl_type::record_types = NULL;
}
+
+ if (glsl_type::interface_types != NULL) {
+ _mesa_hash_table_destroy(glsl_type::interface_types, NULL);
+ glsl_type::interface_types = NULL;
+ }
}
@@ -648,27 +653,28 @@ glsl_type::get_array_instance(const glsl_type *base,
unsigned array_size)
mtx_lock(&glsl_type::mutex);
if (array_types == NULL) {
- array_types = hash_table_ctor(64, hash_table_string_hash,
- hash_table_string_compare);
+ array_types = _mesa_hash_table_create(NULL, _mesa_key_hash_string,
+_mesa_key_string_equal);
}
- const glsl_type *t = (glsl_type *) hash_table_find(array_types, key);
-
- if (t == NULL) {
+ const struct hash_entry *entry = _mesa_hash_table_search(array_types, key);
+ if (entry == NULL) {
mtx_unlock(&glsl_type::mutex);
- t = new glsl_type(base, array_size);
+ const glsl_type *t = new glsl_type(base, array_size);
mtx_lock(&glsl_type::mutex);
- hash_table_insert(array_types, (void *) t, ralloc_strdup(mem_ctx, key));
+ entry = _mesa_hash_table_insert(array_types,
+ ralloc_strdup(mem_ctx, key),
+ (void *) t);
}
- assert(t->base_type == GLSL_TYPE_ARRAY);
- assert(t->length == array_size);
- assert(t->fields.array == base);
+ assert(((glsl_type *)entry->data)->base_type == GLSL_TYPE_ARRAY);
+ assert(((glsl_type *)entry->data)->length == array_size);
+ assert(((glsl_type *)entry->data)->fields.array == base);
mtx_unlock(&glsl_type::mutex);
- return t;
+ return (glsl_type *)entry->data;
}
@@ -722,19 +728,13 @@ glsl_type::record_compare(const glsl_type *b) const
}
-int
+bool
glsl_type::record_key_compare(const void *a, const void *b)
{
const glsl_type *const key1 = (glsl_type *) a;
const glsl_type *const key2 = (glsl_type *) b;
- /* Return zero is the types match (there is zero difference) or non-zero
-* otherwise.
-*/
- if (strcmp(key1->name, key2->name) != 0)
- return 1;
-
- return !key1->record_compare(key2);
+ return strcmp(key1->name, key2->name) == 0 && key1->record_compare(key2);
}
@@ -772,25 +772,27 @@ glsl_type::get_record_instance(const glsl_struct_field
*fields,
mtx_lock(&glsl_type::mutex);
if (record_types == NULL) {
- record_types = hash_table_ctor(64, record_key_hash, record_key_compare);
+ record_types = _mesa_hash_table_create(NULL, record_key_hash,
+ record_key_compare);
}
- const glsl_type *t = (glsl_type *) hash_table_find(record_types, & key);
- if (t == NULL) {
+ const struct hash_entry *entry = _mesa_hash_table_search(record_types,
+&key);
+ if (entry == NULL) {
mtx_unlock(&glsl_type::mutex);
- t = new glsl_type(fields, num_fields, name);
+ const glsl_type *t = new glsl_type(fields, num_fields, name);
mtx_lock(&glsl_type::mutex);
- hash_table_insert(record_types, (void *) t, t);
+ entry = _mesa_hash_table_insert(record_types, t, (void *) t);
}
- assert(t->base_type == GLSL_TYPE_STRUCT);
- assert(t->length == num_fields);
- assert(strcmp(t->name, name) == 0);
+ assert(((glsl_type *)entry->data)->base_type == GLSL_TYPE_STRUCT);
+ assert(((glsl_type *)ent
[Mesa-dev] [PATCH] glsl: remove dead code in a single pass
Currently only one ir assignment is removed for each var in a single
dead code optimisation pass. This means if a var has more than one
assignment, then it requires all the glsl optimisations to be run again
for each additional assignment to be removed.
Another pass is also required to remove the variable itself.
With this change all assignments and the variable are removed in a single
pass.
Some of the arrays of arrays conformance tests that were looping
through 8 dimensions ended up with a var with hundreds of assignments.
This change helps ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1
go from around 3 min 20 sec -> 2 min
ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 went from
around 9 min 20 sec to 7 min 30 sec
I had difficulty getting the public shader-db to give a constent result
with or without this change but the results seemed unchanged at between
15-20 seconds.
---
src/glsl/ir_variable_refcount.cpp | 23 ---
src/glsl/ir_variable_refcount.h | 7 ++-
src/glsl/opt_dead_code.cpp| 30 +-
src/glsl/opt_tree_grafting.cpp| 2 --
4 files changed, 47 insertions(+), 15 deletions(-)
diff --git a/src/glsl/ir_variable_refcount.cpp
b/src/glsl/ir_variable_refcount.cpp
index e4d825c..cc13ac0 100644
--- a/src/glsl/ir_variable_refcount.cpp
+++ b/src/glsl/ir_variable_refcount.cpp
@@ -46,6 +46,16 @@ static void
free_entry(struct hash_entry *entry)
{
ir_variable_refcount_entry *ivre = (ir_variable_refcount_entry *)
entry->data;
+
+ /* Free assignment list */
+ while (!ivre->assign_list.is_empty()) {
+ struct assignment_entry *assignment_entry =
+ exec_node_data(struct assignment_entry,
+ivre->assign_list.head, link);
+ assignment_entry->link.remove();
+ free(assignment_entry);
+ }
+
delete ivre;
}
@@ -59,7 +69,6 @@ ir_variable_refcount_visitor::~ir_variable_refcount_visitor()
ir_variable_refcount_entry::ir_variable_refcount_entry(ir_variable *var)
{
this->var = var;
- assign = NULL;
assigned_count = 0;
declaration = false;
referenced_count = 0;
@@ -125,8 +134,16 @@ ir_variable_refcount_visitor::visit_leave(ir_assignment
*ir)
entry = this->get_variable_entry(ir->lhs->variable_referenced());
if (entry) {
entry->assigned_count++;
- if (entry->assign == NULL)
-entry->assign = ir;
+ /* Build a list for dead code optimisation. Don't bother adding any more
+ * if there are more references then assignments as this means the
+ * variable is used and won't be optimised out.
+ */
+ if (entry->referenced_count == entry->assigned_count) {
+ struct assignment_entry *assignment_entry =
+(struct assignment_entry *)calloc(1, sizeof(*assignment_entry));
+ assignment_entry->assign = ir;
+ entry->assign_list.push_head(&assignment_entry->link);
+ }
}
return visit_continue;
diff --git a/src/glsl/ir_variable_refcount.h b/src/glsl/ir_variable_refcount.h
index c15e8110..1fecd52 100644
--- a/src/glsl/ir_variable_refcount.h
+++ b/src/glsl/ir_variable_refcount.h
@@ -33,13 +33,18 @@
#include "ir_visitor.h"
#include "glsl_types.h"
+struct assignment_entry {
+ exec_node link;
+ ir_assignment *assign;
+};
+
class ir_variable_refcount_entry
{
public:
ir_variable_refcount_entry(ir_variable *var);
ir_variable *var; /* The key: the variable's pointer. */
- ir_assignment *assign; /* An assignment to the variable, if any */
+ exec_list assign_list; /* List of assignments to the variable, if any */
/** Number of times the variable is referenced, including assignments. */
unsigned referenced_count;
diff --git a/src/glsl/opt_dead_code.cpp b/src/glsl/opt_dead_code.cpp
index f45bf5d..6d1f675 100644
--- a/src/glsl/opt_dead_code.cpp
+++ b/src/glsl/opt_dead_code.cpp
@@ -75,22 +75,34 @@ do_dead_code(exec_list *instructions, bool
uniform_locations_assigned)
|| !entry->declaration)
continue;
- if (entry->assign) {
-/* Remove a single dead assignment to the variable we found.
+ if (!entry->assign_list.is_empty()) {
+/* Remove all the dead assignments to the variable we found.
* Don't do so if it's a shader or function output, though.
*/
if (entry->var->data.mode != ir_var_function_out &&
entry->var->data.mode != ir_var_function_inout &&
entry->var->data.mode != ir_var_shader_out) {
- entry->assign->remove();
- progress = true;
- if (debug) {
- printf("Removed assignment to %s@%p\n",
- entry->var->name, (void *) entry->var);
- }
+while (!entry->assign_list.is_empty()) {
+ struct assignment_entry *assignment_entry =
+ exec_node_data(struct assignment_entry,
+ entry->assign_list.head, link);
+
+ as
[Mesa-dev] [PATCH V2] glsl: remove dead code in a single pass
Currently only one ir assignment is removed for each var in a single
dead code optimisation pass. This means if a var has more than one
assignment, then it requires all the glsl optimisations to be run again
for each additional assignment to be removed.
Another pass is also required to remove the variable itself.
With this change all assignments and the variable are removed in a single
pass.
Some of the arrays of arrays conformance tests that were looping
through 8 dimensions ended up with a var with hundreds of assignments.
This change helps ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1
go from around 3 min 20 sec -> 2 min
ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 went from
around 9 min 20 sec to 7 min 30 sec
I had difficulty getting the public shader-db to give a constent result
with or without this change but the results seemed unchanged at between
15-20 seconds.
V2: Dont add assignment ir to the list if its declaration is out of
scope. Add assert to be sure references are counted before assignments.
---
It would be great if someone could check this against the larger shader-db.
The final place where there tests are getting stuck is in constant folding
and propagation.
src/glsl/ir_variable_refcount.cpp | 29 ++---
src/glsl/ir_variable_refcount.h | 13 -
src/glsl/opt_dead_code.cpp| 30 +-
src/glsl/opt_tree_grafting.cpp| 2 --
4 files changed, 59 insertions(+), 15 deletions(-)
diff --git a/src/glsl/ir_variable_refcount.cpp
b/src/glsl/ir_variable_refcount.cpp
index e4d825c..46eb2ca 100644
--- a/src/glsl/ir_variable_refcount.cpp
+++ b/src/glsl/ir_variable_refcount.cpp
@@ -46,6 +46,16 @@ static void
free_entry(struct hash_entry *entry)
{
ir_variable_refcount_entry *ivre = (ir_variable_refcount_entry *)
entry->data;
+
+ /* Free assignment list */
+ while (!ivre->assign_list.is_empty()) {
+ struct assignment_entry *assignment_entry =
+ exec_node_data(struct assignment_entry,
+ivre->assign_list.head, link);
+ assignment_entry->link.remove();
+ free(assignment_entry);
+ }
+
delete ivre;
}
@@ -59,7 +69,6 @@ ir_variable_refcount_visitor::~ir_variable_refcount_visitor()
ir_variable_refcount_entry::ir_variable_refcount_entry(ir_variable *var)
{
this->var = var;
- assign = NULL;
assigned_count = 0;
declaration = false;
referenced_count = 0;
@@ -125,8 +134,22 @@ ir_variable_refcount_visitor::visit_leave(ir_assignment
*ir)
entry = this->get_variable_entry(ir->lhs->variable_referenced());
if (entry) {
entry->assigned_count++;
- if (entry->assign == NULL)
-entry->assign = ir;
+
+ /**
+ * Build a list for dead code optimisation. Dont add assingment if it
+ * was declared out of scope (outside the instruction stream). Also dont
+ * bother adding any more to the list if there are more references then
+ * assignments as this means the variable is used and won't be optimised
+ * out.
+ */
+ assert(entry->referenced_count >= entry->assigned_count);
+ if (entry->declaration &&
+ entry->referenced_count == entry->assigned_count) {
+ struct assignment_entry *assignment_entry =
+(struct assignment_entry *)calloc(1, sizeof(*assignment_entry));
+ assignment_entry->assign = ir;
+ entry->assign_list.push_head(&assignment_entry->link);
+ }
}
return visit_continue;
diff --git a/src/glsl/ir_variable_refcount.h b/src/glsl/ir_variable_refcount.h
index c15e8110..5c74c31 100644
--- a/src/glsl/ir_variable_refcount.h
+++ b/src/glsl/ir_variable_refcount.h
@@ -33,13 +33,24 @@
#include "ir_visitor.h"
#include "glsl_types.h"
+struct assignment_entry {
+ exec_node link;
+ ir_assignment *assign;
+};
+
class ir_variable_refcount_entry
{
public:
ir_variable_refcount_entry(ir_variable *var);
ir_variable *var; /* The key: the variable's pointer. */
- ir_assignment *assign; /* An assignment to the variable, if any */
+
+ /**
+* List of assignments to the variable, if any.
+* This is intended to be used for dead code optimisation and may
+* not be a complete list.
+*/
+ exec_list assign_list;
/** Number of times the variable is referenced, including assignments. */
unsigned referenced_count;
diff --git a/src/glsl/opt_dead_code.cpp b/src/glsl/opt_dead_code.cpp
index f45bf5d..6d1f675 100644
--- a/src/glsl/opt_dead_code.cpp
+++ b/src/glsl/opt_dead_code.cpp
@@ -75,22 +75,34 @@ do_dead_code(exec_list *instructions, bool
uniform_locations_assigned)
|| !entry->declaration)
continue;
- if (entry->assign) {
-/* Remove a single dead assignment to the variable we found.
+ if (!entry->assign_list.is_empty()) {
+/* Remove all the dead assignments to the variable we found.
* Don't do so if it's a shader or function output, though.
Re: [Mesa-dev] [PATCH V2] glsl: remove dead code in a single pass
On Sun, 2015-07-12 at 15:47 +1000, Timothy Arceri wrote:
> Currently only one ir assignment is removed for each var in a single
> dead code optimisation pass. This means if a var has more than one
> assignment, then it requires all the glsl optimisations to be run again
> for each additional assignment to be removed.
> Another pass is also required to remove the variable itself.
>
> With this change all assignments and the variable are removed in a single
> pass.
>
> Some of the arrays of arrays conformance tests that were looping
> through 8 dimensions ended up with a var with hundreds of assignments.
>
> This change helps ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1
> go from around 3 min 20 sec -> 2 min
>
> ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 went from
> around 9 min 20 sec to 7 min 30 sec
>
> I had difficulty getting the public shader-db to give a constent result
> with or without this change but the results seemed unchanged at between
> 15-20 seconds.
>
> V2: Dont add assignment ir to the list if its declaration is out of
> scope.
On second thought I'm not 100% sure this is safe to check like this in the
refcount visitor I'll probably drop this version 2 and just stick with the
original if people think its ok.
> Add assert to be sure references are counted before assignments.
> ---
> It would be great if someone could check this against the larger shader-db.
>
> The final place where there tests are getting stuck is in constant folding
> and propagation.
>
> src/glsl/ir_variable_refcount.cpp | 29 ++---
> src/glsl/ir_variable_refcount.h | 13 -
> src/glsl/opt_dead_code.cpp| 30 +-
> src/glsl/opt_tree_grafting.cpp| 2 --
> 4 files changed, 59 insertions(+), 15 deletions(-)
>
> diff --git a/src/glsl/ir_variable_refcount.cpp
> b/src/glsl/ir_variable_refcount.cpp
> index e4d825c..46eb2ca 100644
> --- a/src/glsl/ir_variable_refcount.cpp
> +++ b/src/glsl/ir_variable_refcount.cpp
> @@ -46,6 +46,16 @@ static void
> free_entry(struct hash_entry *entry)
> {
> ir_variable_refcount_entry *ivre = (ir_variable_refcount_entry *) entry
> ->data;
> +
> + /* Free assignment list */
> + while (!ivre->assign_list.is_empty()) {
> + struct assignment_entry *assignment_entry =
> + exec_node_data(struct assignment_entry,
> +ivre->assign_list.head, link);
> + assignment_entry->link.remove();
> + free(assignment_entry);
> + }
> +
> delete ivre;
> }
>
> @@ -59,7 +69,6 @@
> ir_variable_refcount_visitor::~ir_variable_refcount_visitor()
> ir_variable_refcount_entry::ir_variable_refcount_entry(ir_variable *var)
> {
> this->var = var;
> - assign = NULL;
> assigned_count = 0;
> declaration = false;
> referenced_count = 0;
> @@ -125,8 +134,22 @@ ir_variable_refcount_visitor::visit_leave(ir_assignment
> *ir)
> entry = this->get_variable_entry(ir->lhs->variable_referenced());
> if (entry) {
>entry->assigned_count++;
> - if (entry->assign == NULL)
> - entry->assign = ir;
> +
> + /**
> + * Build a list for dead code optimisation. Dont add assingment if it
> + * was declared out of scope (outside the instruction stream). Also
> dont
> + * bother adding any more to the list if there are more references
> then
> + * assignments as this means the variable is used and won't be
> optimised
> + * out.
> + */
> + assert(entry->referenced_count >= entry->assigned_count);
> + if (entry->declaration &&
> + entry->referenced_count == entry->assigned_count) {
> + struct assignment_entry *assignment_entry =
> +(struct assignment_entry *)calloc(1,
> sizeof(*assignment_entry));
> + assignment_entry->assign = ir;
> + entry->assign_list.push_head(&assignment_entry->link);
> + }
> }
>
> return visit_continue;
> diff --git a/src/glsl/ir_variable_refcount.h
> b/src/glsl/ir_variable_refcount.h
> index c15e8110..5c74c31 100644
> --- a/src/glsl/ir_variable_refcount.h
> +++ b/src/glsl/ir_variable_refcount.h
> @@ -33,13 +33,24 @@
> #include "ir_visitor.h"
> #include "glsl_types.h"
>
> +struct assignment_entry {
> + exec_node link;
> + ir_assignment *assign;
> +};
> +
> class ir_variable_refcount_entry
> {
> public:
> ir_variable_refcount_entry(ir_variable *var);
>
> ir_variable *var; /* The key: the variable's pointer. */
> - ir
Re: [Mesa-dev] [PATCH] glsl: remove dead code in a single pass
On Sun, 2015-07-12 at 13:38 +0200, Thomas Helland wrote: > 2015-07-12 1:17 GMT+02:00 Timothy Arceri : > > snip > > > Hi Timothy, > > Just gave your patch a whirl through my shader-db. > Three runs on master and three runs with your path seems > to cut my shader-db run from aprox 117 secs to 112. > (I didn't do any statistics, just a quick look at the results.) > Also there is no changes in instruction counts, > which is to be expected, but nice to confirm. > Let me know if you want more thorough statistics. > If you want you can strap my "Tested-By" on this. > > Thomas Thanks for giving this a run Thomas :) I'm happy with these numbers but will wait and see if others want more precise numbers. Also thanks for confirming the instruction count remains unchanged. I'll add your Tested-by and shader-db numbers to the commit message. Thanks again, Tim ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: free interface_types and replace old hash_table uses
On Mon, 2015-07-13 at 22:19 -0700, Kenneth Graunke wrote:
> On Monday, July 13, 2015 11:21:05 AM Iago Toral wrote:
> > On Sat, 2015-07-11 at 10:13 +1000, Timothy Arceri wrote:
> > > @@ -648,27 +653,28 @@ glsl_type::get_array_instance(const glsl_type
> > > *base, unsigned array_size)
> > > mtx_lock(&glsl_type::mutex);
> > >
> > > if (array_types == NULL) {
> > > - array_types = hash_table_ctor(64, hash_table_string_hash,
> > > - hash_table_string_compare);
> > > + array_types = _mesa_hash_table_create(NULL,
> > > _mesa_key_hash_string,
> > > +_mesa_key_string_equal);
> > > }
> > >
> > > - const glsl_type *t = (glsl_type *) hash_table_find(array_types,
> > > key);
> > > -
> > > - if (t == NULL) {
> > > + const struct hash_entry *entry =
> > > _mesa_hash_table_search(array_types, key);
> > > + if (entry == NULL) {
> > >mtx_unlock(&glsl_type::mutex);
> > > - t = new glsl_type(base, array_size);
> > > + const glsl_type *t = new glsl_type(base, array_size);
> > >mtx_lock(&glsl_type::mutex);
> > >
> > > - hash_table_insert(array_types, (void *) t, ralloc_strdup(mem_ctx,
> > > key));
> > > + entry = _mesa_hash_table_insert(array_types,
> > > + ralloc_strdup(mem_ctx, key),
> > > + (void *) t);
> > > }
> > >
> > > - assert(t->base_type == GLSL_TYPE_ARRAY);
> > > - assert(t->length == array_size);
> > > - assert(t->fields.array == base);
> > > + assert(((glsl_type *)entry->data)->base_type == GLSL_TYPE_ARRAY);
> > > + assert(((glsl_type *)entry->data)->length == array_size);
> > > + assert(((glsl_type *)entry->data)->fields.array == base);
> >
> > Other parts of this file put a blank between the type cast and the
> > variable, so I would add that here (and in all other places where you
> > cast entry to glsl_type* in this patch).
>
> Or...why not continue to have a local variable t, and just set t =
> entry->data? Then these could all stay the same, and there would be
> less casting.
I did have it like that but in my opinion it just looked messy. 2 extra lines
vs 3 extra casts all of which are in asserts(), it felt like I was making the
surrounding code worse for the sake of the asserts.
I've pushed this already so I guess it doesn't matter now anyway.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: replace null check with assert
This was added in 54f583a20 since then error handling has improved.
The test this was added to fix now fails earlier since 01822706ec
---
src/glsl/ir_constant_expression.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/glsl/ir_constant_expression.cpp
b/src/glsl/ir_constant_expression.cpp
index 171b8e9..5732867 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -1857,9 +1857,7 @@ ir_swizzle::constant_expression_value(struct hash_table
*variable_context)
ir_constant *
ir_dereference_variable::constant_expression_value(struct hash_table
*variable_context)
{
- /* This may occur during compile and var->type is glsl_type::error_type */
- if (!var)
- return NULL;
+ assert(var);
/* Give priority to the context hashtable, if it exists */
if (variable_context) {
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Seeking advice speeding up glsl optimisation passes for AoA
Hi guys, As I've mentioned a couple of times in previous patches some of the cts AoA tests are taking very long time to compile. This is due to excessive optimisation passes mainly in the glsl optimisations (there are some slowdowns in the intel backend too but these seemed to go away when I tried the new nir vec4 backend). I fixed part of the problem with this patch to do the dead code elimination in a single pass [1]. These excessive passes exist in normal shaders but its generally not an issue as the number of passes is generally quite low, and inexpensive. However when you have an 8 dimensional array constantly walking this becomes quite expensive. The remaining issue I'm seeking some advice for is with constant propagation/folding. It seems for interators used in loops you can get into a situation where an optimisation pass is needed for each loop iteration in order to make all values of the iterator constant. I didn't have look too find some real world examples of this in the public shader-db. For example here is it happening for a Unity shader: -- Linker Pass 1 -- opt_function_inlining progress opt_dead_functions progress opt_copy_propagation_elements progress opt_tree_grafting progress opt_constant_folding progress opt_algebraic progress -- Linker Pass 1 -- constant prop : l_3, Value : 0 constant prop : l_3, Value : 0 constant prop : l_3, Value : 0 constant prop : l_3, Value : 0 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 2 -- constant prop : l_3, Value : 1 constant prop : l_3, Value : 1 constant prop : l_3, Value : 1 constant prop : l_3, Value : 1 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 3 -- constant prop : l_3, Value : 2 constant prop : l_3, Value : 2 constant prop : l_3, Value : 2 constant prop : l_3, Value : 2 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 4 -- constant prop : l_3, Value : 3 constant prop : l_3, Value : 3 constant prop : l_3, Value : 3 constant prop : l_3, Value : 3 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 5 -- constant prop : l_3, Value : 4 constant prop : l_3, Value : 4 constant prop : l_3, Value : 4 constant prop : l_3, Value : 4 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 6 -- constant prop : l_3, Value : 5 constant prop : l_3, Value : 5 constant prop : l_3, Value : 5 constant prop : l_3, Value : 5 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 7 -- constant prop : l_3, Value : 6 constant prop : l_3, Value : 6 constant prop : l_3, Value : 6 constant prop : l_3, Value : 6 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 8 -- constant prop : l_3, Value : 7 constant prop : l_3, Value : 7 constant prop : l_3, Value : 7 constant prop : l_3, Value : 7 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 9 -- constant prop : l_3, Value : 8 constant prop : l_3, Value : 8 constant prop : l_3, Value : 8 constant prop : l_3, Value : 8 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 10 -- constant prop : l_3, Value : 9 constant prop : l_3, Value : 9 constant prop : l_3, Value : 9 constant prop : l_3, Value : 9 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 11 -- constant prop : l_3, Value : 10 constant prop : l_3, Value : 10 constant prop : l_3, Value : 10 constant prop : l_3, Value : 10 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 12 -- constant prop : l_3, Value : 11 constant prop : l_3, Value : 11 constant prop : l_3, Value : 11 constant prop : l_3, Value : 11 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 13 -- constant prop : l_3, Value : 12 constant prop : l_3, Value : 12 constant prop : l_3, Value : 12 constant prop : l_3, Value : 12 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 14 -- constant prop : l_3, Value : 13 constant prop : l_3, Value : 13 constant prop : l_3, Value : 13 constant prop : l_3, Value : 13 opt_constant_propagation progress opt_constant_folding progress -- Linker Pass 15 -- constant prop : l_3, Value : 14 constant prop : l_3, Value : 14 constant prop : l_3, Value : 14 constant prop : l_3, Value : 14 opt_
Re: [Mesa-dev] Seeking advice speeding up glsl optimisation passes for AoA
On Wed, 2015-07-15 at 16:42 +1000, Timothy Arceri wrote: > Hi guys, > > As I've mentioned a couple of times in previous patches some of the cts AoA > tests are taking very long time to compile. This is due to excessive > optimisation passes mainly in the glsl optimisations (there are some > slowdowns > in the intel backend too but these seemed to go away when I tried the new > nir > vec4 backend). > > I fixed part of the problem with this patch to do the dead code elimination > in > a single pass [1]. > These excessive passes exist in normal shaders but its generally not an > issue > as the number of passes is generally quite low, and inexpensive. However > when > you have an 8 dimensional array constantly walking this becomes quite > expensive. > > The remaining issue I'm seeking some advice for is with constant > propagation/folding. > > It seems for interators used in loops you can get into a situation where an > optimisation pass is needed for each loop iteration in order to make all > values of the iterator constant. > > I didn't have look too find some real world examples of this in the public > shader-db. For example here is it happening for a Unity shader: > > -- Linker Pass 1 -- > opt_function_inlining progress > opt_dead_functions progress > opt_copy_propagation_elements progress > opt_tree_grafting progress > opt_constant_folding progress > opt_algebraic progress > -- Linker Pass 1 -- > constant prop : l_3, Value : 0 > constant prop : l_3, Value : 0 > constant prop : l_3, Value : 0 > constant prop : l_3, Value : 0 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 2 -- > constant prop : l_3, Value : 1 > constant prop : l_3, Value : 1 > constant prop : l_3, Value : 1 > constant prop : l_3, Value : 1 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 3 -- > constant prop : l_3, Value : 2 > constant prop : l_3, Value : 2 > constant prop : l_3, Value : 2 > constant prop : l_3, Value : 2 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 4 -- > constant prop : l_3, Value : 3 > constant prop : l_3, Value : 3 > constant prop : l_3, Value : 3 > constant prop : l_3, Value : 3 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 5 -- > constant prop : l_3, Value : 4 > constant prop : l_3, Value : 4 > constant prop : l_3, Value : 4 > constant prop : l_3, Value : 4 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 6 -- > constant prop : l_3, Value : 5 > constant prop : l_3, Value : 5 > constant prop : l_3, Value : 5 > constant prop : l_3, Value : 5 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 7 -- > constant prop : l_3, Value : 6 > constant prop : l_3, Value : 6 > constant prop : l_3, Value : 6 > constant prop : l_3, Value : 6 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 8 -- > constant prop : l_3, Value : 7 > constant prop : l_3, Value : 7 > constant prop : l_3, Value : 7 > constant prop : l_3, Value : 7 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 9 -- > constant prop : l_3, Value : 8 > constant prop : l_3, Value : 8 > constant prop : l_3, Value : 8 > constant prop : l_3, Value : 8 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 10 -- > constant prop : l_3, Value : 9 > constant prop : l_3, Value : 9 > constant prop : l_3, Value : 9 > constant prop : l_3, Value : 9 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 11 -- > constant prop : l_3, Value : 10 > constant prop : l_3, Value : 10 > constant prop : l_3, Value : 10 > constant prop : l_3, Value : 10 > opt_constant_propagation progress > opt_constant_folding progress > -- Linker Pass 12 -- > constant prop : l_3, Value : 11 > constant prop : l_3, Value : 11 > constant prop : l_3, Value : 11 > constant prop : l_3, Value : 11 > opt_constant_propagation progress > opt_constant_folding progress > -
Re: [Mesa-dev] Seeking advice speeding up glsl optimisation passes for AoA
On Wed, 2015-07-15 at 11:53 -0700, Eric Anholt wrote: > Timothy Arceri writes: > > > Hi guys, > > > > As I've mentioned a couple of times in previous patches some of the cts > > AoA > > tests are taking very long time to compile. This is due to excessive > > optimisation passes mainly in the glsl optimisations (there are some > > slowdowns > > in the intel backend too but these seemed to go away when I tried the new > > nir > > vec4 backend). > > > > I fixed part of the problem with this patch to do the dead code > > elimination in > > a single pass [1]. > > These excessive passes exist in normal shaders but its generally not an > > issue > > as the number of passes is generally quite low, and inexpensive. However > > when > > you have an 8 dimensional array constantly walking this becomes quite > > expensive. > > > > The remaining issue I'm seeking some advice for is with constant > > propagation/folding. > > > > It seems for interators used in loops you can get into a situation where > > an > > optimisation pass is needed for each loop iteration in order to make all > > values of the iterator constant. > > > > I didn't have look too find some real world examples of this in the public > > shader-db. For example here is it happening for a Unity shader: > > How about if we just disable the GLSL IR constant prop pass when NIR is > enabled? Thanks for the suggestion, I was hoping that would be the answer but seems we can't do it yet. However I seem to have a solution to my problem which was quite simple in the end. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/12] glsl: automake: remove custom AM_V_LEX/YACC
On Fri, 2015-07-17 at 19:35 +0100, Emil Velikov wrote: > On 17 July 2015 at 19:02, Matt Turner wrote: > > On Fri, Jul 17, 2015 at 10:29 AM, Emil Velikov > > wrote: > > > We already use AM_V_GEN in src/mesa for lex/yacc. Plus we use the > > > generic macro for python generated sources as well. > > > > I'm in favor of removing "extras" when they cause problems, but I'm > > not aware of any -- most of the lines git blame to the original > > commit. > > > It is strange that we have these in one place but not the other. As-is > It's a bit confusing - do you have any objections if we keep them and > add a pair in src/mesa ? > > > Maybe we should do a survey of required automake versions and see if > > we can require >= 1.12? From before, Red Hat wanted 1.11 support for > > RHEL 5? I think 1.11 support is for RHEL 6. Mesa's automake version is already to high for RHEL 5 something I checked when bumping the gcc version. > Would be nice to nuke them, but as you mentioned 1.11 is still used > iirc (from a quick chat with Dave (?) a month or so ago). > > Cheers > Emil > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/16] mesa: remove overlapping array index validation and extra uniform location lookup
This change removes multiple functions designed to validate an array
subscript and replaces them with a call to a single function.
The change also means that validation is now only done once and the index
is retrived at the same time, as a result the getUniformLocation code can
be simplified saving an extra hash table lookup (and yet another
validation call).
This chage also fixes some tests in:
ES31-CTS.program_interface_query.uniform
V2: Fix bounds checks for program input/output, split unrelated comment fix
and _mesa_get_uniform_location() removal into their own patch.
---
src/mesa/main/program_resource.c | 66 ++---
src/mesa/main/shader_query.cpp | 204 ---
src/mesa/main/shaderapi.h| 7 +-
src/mesa/main/uniforms.c | 7 +-
4 files changed, 100 insertions(+), 184 deletions(-)
diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index d857b84..0a306b8 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -173,32 +173,12 @@ is_xfb_marker(const char *str)
return false;
}
-/**
- * Checks if given name index is legal for GetProgramResourceIndex,
- * check is written to be compatible with GL_ARB_array_of_arrays.
- */
-static bool
-valid_program_resource_index_name(const GLchar *name)
-{
- const char *array = strstr(name, "[");
- const char *close = strrchr(name, ']');
-
- /* Not array, no need for the check. */
- if (!array)
- return true;
-
- /* Last array index has to be zero. */
- if (!close || *--close != '0')
- return false;
-
- return true;
-}
-
GLuint GLAPIENTRY
_mesa_GetProgramResourceIndex(GLuint program, GLenum programInterface,
const GLchar *name)
{
GET_CURRENT_CONTEXT(ctx);
+ unsigned array_index;
struct gl_program_resource *res;
struct gl_shader_program *shProg =
_mesa_lookup_shader_program_err(ctx, program,
@@ -221,12 +201,9 @@ _mesa_GetProgramResourceIndex(GLuint program, GLenum
programInterface,
case GL_PROGRAM_OUTPUT:
case GL_UNIFORM:
case GL_TRANSFORM_FEEDBACK_VARYING:
- /* Validate name syntax for array variables */
- if (!valid_program_resource_index_name(name))
- return GL_INVALID_INDEX;
- /* fall-through */
case GL_UNIFORM_BLOCK:
- res = _mesa_program_resource_find_name(shProg, programInterface, name);
+ res = _mesa_program_resource_find_name(shProg, programInterface, name,
+ &array_index, true);
if (!res)
return GL_INVALID_INDEX;
@@ -300,36 +277,6 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum
programInterface,
propCount, props, bufSize, length, params);
}
-/**
- * Function verifies syntax of given name for GetProgramResourceLocation
- * and GetProgramResourceLocationIndex for the following cases:
- *
- * "array element portion of a string passed to GetProgramResourceLocation
- * or GetProgramResourceLocationIndex must not have, a "+" sign, extra
- * leading zeroes, or whitespace".
- *
- * Check is written to be compatible with GL_ARB_array_of_arrays.
- */
-static bool
-invalid_array_element_syntax(const GLchar *name)
-{
- char *first = strchr(name, '[');
- char *last = strrchr(name, '[');
-
- if (!first)
- return false;
-
- /* No '+' or ' ' allowed anywhere. */
- if (strchr(first, '+') || strchr(first, ' '))
- return true;
-
- /* Check that last array index is 0. */
- if (last[1] == '0' && last[2] != ']')
- return true;
-
- return false;
-}
-
static struct gl_shader_program *
lookup_linked_program(GLuint program, const char *caller)
{
@@ -356,7 +303,7 @@ _mesa_GetProgramResourceLocation(GLuint program, GLenum
programInterface,
struct gl_shader_program *shProg =
lookup_linked_program(program, "glGetProgramResourceLocation");
- if (!shProg || !name || invalid_array_element_syntax(name))
+ if (!shProg || !name)
return -1;
/* Validate programInterface. */
@@ -383,7 +330,8 @@ _mesa_GetProgramResourceLocation(GLuint program, GLenum
programInterface,
_mesa_lookup_enum_by_nr(programInterface), name);
}
- return _mesa_program_resource_location(shProg, programInterface, name);
+ return _mesa_program_resource_location(shProg, programInterface, name,
+ true);
}
/**
@@ -397,7 +345,7 @@ _mesa_GetProgramResourceLocationIndex(GLuint program,
GLenum programInterface,
struct gl_shader_program *shProg =
lookup_linked_program(program, "glGetProgramResourceLocationIndex");
- if (!shProg || !name || invalid_array_element_syntax(name))
+ if (!shProg || !name)
return -1;
/* From the GL_ARB_program_interface_query spec:
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 0473c2e..3b08ea1 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/me
[Mesa-dev] [PATCH 01/16] glsl: check for leading zeros in array index validation
--- src/glsl/linker.cpp | 4 1 file changed, 4 insertions(+) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index b7a783c..cb679bd 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -462,6 +462,10 @@ parse_program_resource_name(const GLchar *name, if (array_index < 0) return -1; + /* Check for leading zero */ + if (name[i] == '0' && name[i+1] != ']') + return -1; + *out_base_name_end = name + (i - 1); return array_index; } -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/16] mesa: fix incorrect comment
--- src/mesa/main/shader_query.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index a6246a3..0473c2e 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -787,8 +787,6 @@ program_resource_location(struct gl_shader_program *shProg, /** * Function implements following location queries: - *glGetAttribLocation - *glGetFragDataLocation *glGetUniformLocation */ GLint -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/16] mesa: fix active sampler conflict validation
The type stored in gl_uniform_storage is the type of a single array
element not the array type so size was always 1.
V2: Dont validate sampler units pointing to 0
---
src/mesa/main/uniform_query.cpp | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index 680ba16..036530e 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -1026,18 +1026,23 @@ _mesa_sampler_uniforms_pipeline_are_valid(struct
gl_pipeline_object *pipeline)
for (unsigned i = 0; i < shProg[idx]->NumUniformStorage; i++) {
const struct gl_uniform_storage *const storage =
&shProg[idx]->UniformStorage[i];
- const glsl_type *const t = (storage->type->is_array())
-? storage->type->fields.array : storage->type;
- if (!t->is_sampler())
+ if (!storage->type->is_sampler())
continue;
active_samplers++;
- const unsigned count = MAX2(1, storage->type->array_size());
+ const unsigned count = MAX2(1, storage->array_elements);
for (unsigned j = 0; j < count; j++) {
const unsigned unit = storage->storage[j].i;
+/* FIXME: Samplers are initialized to 0 and Mesa doesn't do a
+ * great job of eliminating unused uniforms currently so for now
+ * don't throw an error if two sampler types both point to 0.
+ */
+if (unit == 0)
+ continue;
+
/* The types of the samplers associated with a particular texture
* unit must be an exact match. Page 74 (page 89 of the PDF) of
* the OpenGL 3.3 core spec says:
@@ -1047,13 +1052,14 @@ _mesa_sampler_uniforms_pipeline_are_valid(struct
gl_pipeline_object *pipeline)
* program object."
*/
if (unit_types[unit] == NULL) {
- unit_types[unit] = t;
-} else if (unit_types[unit] != t) {
+ unit_types[unit] = storage->type;
+} else if (unit_types[unit] != storage->type) {
pipeline->InfoLog =
ralloc_asprintf(pipeline,
"Texture unit %d is accessed both as %s "
"and %s",
- unit, unit_types[unit]->name, t->name);
+ unit, unit_types[unit]->name,
+ storage->type->name);
return false;
}
}
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/16] glsl: calculate component size for arrays of arrays when varying packing disabled
Signed-off-by: Timothy Arceri
Reviewed-by: Ilia Mirkin
---
src/glsl/link_varyings.cpp | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 020842a..ca5f8d3 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -886,9 +886,18 @@ varying_matches::record(ir_variable *producer_var,
ir_variable *consumer_var)
this->matches[this->num_matches].packing_order
= this->compute_packing_order(var);
if (this->disable_varying_packing) {
- unsigned slots = var->type->is_array()
- ? (var->type->length * var->type->fields.array->matrix_columns)
- : var->type->matrix_columns;
+ unsigned slots;
+ if (var->type->is_array()) {
+ const glsl_type *type = var->type;
+ slots = 1;
+ while (type->is_array()) {
+slots *= type->length;
+type = type->fields.array;
+ }
+ slots *= type->matrix_columns;
+ } else {
+ slots = var->type->matrix_columns;
+ }
this->matches[this->num_matches].num_components = 4 * slots;
} else {
this->matches[this->num_matches].num_components
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/16] glsl: Add support for linking uniform arrays of arrays
V2: Handle arrays of arrays in the same way structures are handled
The ARB_arrays_of_arrays spec doesn't give very many details on how
AoA uniforms are intended to be implemented. However in the
ARB_program_interface_query spec there are details that show AoA are
intended to be handled in a similar way to structs.
Issues 7 from the ARB_program_interface_query spec:
We define rules consistent with our enumeration rules for
other complex types. For existing one-dimensional arrays, we enumerate
a single entry if the array is an array of basic types, or separate
entries for each array element if the array is an array of structures.
We follow similar rules here. For a uniform array such as:
uniform vec4 a[5][4][3];
we enumerate twenty different entries ("a[0][0][0]" through
"a[4][3][0]"), each of which is treated as an array with three elements.
This is morally equivalent to what you'd get if you worked around the
limitation in current GLSL via:
struct ArrayBottom { vec4 c[3]; };
struct ArrayMid{ ArrayBottom b[3]; };
uniform ArrayMid a[5];
which would enumerate "a[0].b[0].c[0]" through "a[4].b[3].c[0]".
---
src/glsl/link_uniform_initializers.cpp | 4 +++-
src/glsl/link_uniforms.cpp | 16 ++--
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/glsl/link_uniform_initializers.cpp
b/src/glsl/link_uniform_initializers.cpp
index 204acfa..d0e68ad 100644
--- a/src/glsl/link_uniform_initializers.cpp
+++ b/src/glsl/link_uniform_initializers.cpp
@@ -164,6 +164,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program
*prog,
const char *name, const glsl_type *type,
ir_constant *val, unsigned int boolean_true)
{
+ const glsl_type *t_without_array = type->without_array();
if (type->is_record()) {
ir_constant *field_constant;
@@ -178,7 +179,8 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program
*prog,
field_constant = (ir_constant *)field_constant->next;
}
return;
- } else if (type->is_array() && type->fields.array->is_record()) {
+ } else if (t_without_array->is_record() ||
+ (type->is_array() && type->fields.array->is_array())) {
const glsl_type *const element_type = type->fields.array;
for (unsigned int i = 0; i < type->length; i++) {
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 2d50b9b..c6dfa88 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -49,8 +49,8 @@ values_for_type(const glsl_type *type)
{
if (type->is_sampler()) {
return 1;
- } else if (type->is_array() && type->fields.array->is_sampler()) {
- return type->array_size();
+ } else if (type->is_array()) {
+ return type->array_size() * values_for_type(type->fields.array);
} else {
return type->component_slots();
}
@@ -137,7 +137,8 @@ program_resource_visitor::process(ir_variable *var)
*/
recursion(var->type, &name, strlen(name), row_major, NULL, false);
ralloc_free(name);
- } else if (t->without_array()->is_record()) {
+ } else if (t_without_array->is_record() ||
+ (t->is_array() && t->fields.array->is_array())) {
char *name = ralloc_strdup(NULL, var->name);
recursion(var->type, &name, strlen(name), row_major, NULL, false);
ralloc_free(name);
@@ -214,7 +215,8 @@ program_resource_visitor::recursion(const glsl_type *t,
char **name,
this->leave_record(t, *name, row_major);
}
} else if (t->without_array()->is_record()
- || t->without_array()->is_interface()) {
+ || t->without_array()->is_interface()
+ || (t->is_array() && t->fields.array->is_array())) {
if (record_type == NULL && t->fields.array->is_record())
record_type = t->fields.array;
@@ -565,6 +567,7 @@ private:
{
assert(!type->without_array()->is_record());
assert(!type->without_array()->is_interface());
+ assert(!(type->is_array() && type->fields.array->is_array()));
unsigned id;
bool found = this->map->get(id, name);
@@ -636,7 +639,7 @@ private:
if (type->is_array()) {
this->uniforms[id].array_stride =
- glsl_align(type->fields.array->std140_size(row_major), 16);
+ glsl_align(type->without_array()->std140_size(row_major), 16);
} else {
this->uniforms[id].array_stride = 0;
}
@@ -775,7 +778,8 @@ link_update_uniform_buffer_variables(struct gl_shader
*shader)
if (var->type->is_record()) {
sentinel = '.';
- } else if (var->type->without_array()->is_record()) {
+ } else if (var->type->is_array() && (var->type->fields.array->is_array()
+ || var->type->fields.array->is_record())) {
sentinel = '[';
}
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists
[Mesa-dev] [PATCH 05/16] glsl: set stage flag for structs and arrays in resource list
This fixes the remaining failing tests in:
ES31-CTS.program_interface_query.uniform-types
---
src/glsl/linker.cpp | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index cb679bd..dc8c873 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -2648,9 +2648,19 @@ build_stageref(struct gl_shader_program *shProg, const
char *name)
*/
foreach_in_list(ir_instruction, node, sh->ir) {
ir_variable *var = node->as_variable();
- if (var && strcmp(var->name, name) == 0) {
-stages |= (1 << i);
-break;
+ if (var) {
+unsigned baselen = strlen(var->name);
+if (strncmp(var->name, name, baselen) == 0) {
+ /* Check for exact name matches but also check for arrays and
+* structs.
+*/
+ if (name[baselen] == '\0' ||
+ name[baselen] == '[' ||
+ name[baselen] == '.') {
+ stages |= (1 << i);
+ break;
+ }
+}
}
}
}
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/16] mesa: remove now unused _mesa_get_uniform_location
---
src/mesa/main/uniform_query.cpp | 75 -
src/mesa/main/uniforms.h| 4 ---
2 files changed, 79 deletions(-)
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index cab5083..680ba16 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -978,81 +978,6 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct
gl_shader_program *shProg,
}
-/**
- * Called via glGetUniformLocation().
- *
- * Returns the uniform index into UniformStorage (also the
- * glGetActiveUniformsiv uniform index), and stores the referenced
- * array offset in *offset, or GL_INVALID_INDEX (-1).
- */
-extern "C" unsigned
-_mesa_get_uniform_location(struct gl_shader_program *shProg,
- const GLchar *name,
- unsigned *out_offset)
-{
- /* Page 80 (page 94 of the PDF) of the OpenGL 2.1 spec says:
-*
-* "The first element of a uniform array is identified using the
-* name of the uniform array appended with "[0]". Except if the last
-* part of the string name indicates a uniform array, then the
-* location of the first element of that array can be retrieved by
-* either using the name of the uniform array, or the name of the
-* uniform array appended with "[0]"."
-*
-* Note: since uniform names are not allowed to use whitespace, and array
-* indices within uniform names are not allowed to use "+", "-", or leading
-* zeros, it follows that each uniform has a unique name up to the possible
-* ambiguity with "[0]" noted above. Therefore we don't need to worry
-* about mal-formed inputs--they will properly fail when we try to look up
-* the uniform name in shProg->UniformHash.
-*/
-
- const GLchar *base_name_end;
- long offset = parse_program_resource_name(name, &base_name_end);
- bool array_lookup = offset >= 0;
- char *name_copy;
-
- if (array_lookup) {
- name_copy = (char *) malloc(base_name_end - name + 1);
- memcpy(name_copy, name, base_name_end - name);
- name_copy[base_name_end - name] = '\0';
- } else {
- name_copy = (char *) name;
- offset = 0;
- }
-
- unsigned location = 0;
- const bool found = shProg->UniformHash->get(location, name_copy);
-
- assert(!found
- || strcmp(name_copy, shProg->UniformStorage[location].name) == 0);
-
- /* Free the temporary buffer *before* possibly returning an error.
-*/
- if (name_copy != name)
- free(name_copy);
-
- if (!found)
- return GL_INVALID_INDEX;
-
- /* If the uniform is built-in, fail. */
- if (shProg->UniformStorage[location].builtin)
- return GL_INVALID_INDEX;
-
- /* If the uniform is an array, fail if the index is out of bounds.
-* (A negative index is caught above.) This also fails if the uniform
-* is not an array, but the user is trying to index it, because
-* array_elements is zero and offset >= 0.
-*/
- if (array_lookup
- && offset >= (long) shProg->UniformStorage[location].array_elements) {
- return GL_INVALID_INDEX;
- }
-
- *out_offset = offset;
- return location;
-}
-
extern "C" bool
_mesa_sampler_uniforms_are_valid(const struct gl_shader_program *shProg,
char *errMsg, size_t errMsgLength)
diff --git a/src/mesa/main/uniforms.h b/src/mesa/main/uniforms.h
index bd7b05e..e62eaa5 100644
--- a/src/mesa/main/uniforms.h
+++ b/src/mesa/main/uniforms.h
@@ -343,10 +343,6 @@ void GLAPIENTRY
_mesa_ProgramUniformMatrix4x3dv(GLuint program, GLint location, GLsizei count,
GLboolean transpose, const GLdouble *value);
-unsigned
-_mesa_get_uniform_location(struct gl_shader_program *shProg,
- const GLchar *name, unsigned *offset);
-
void
_mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shader_program,
GLint location, GLsizei count,
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/16] glsl: fix binding validation for interface blocks
---
src/glsl/ast_to_hir.cpp | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 1e3686c..341f9c4 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2041,10 +2041,10 @@ validate_matrix_layout_for_type(struct
_mesa_glsl_parse_state *state,
static bool
validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
YYLTYPE *loc,
- ir_variable *var,
+ const glsl_type *type,
const ast_type_qualifier *qual)
{
- if (var->data.mode != ir_var_uniform) {
+ if (!qual->flags.q.uniform) {
_mesa_glsl_error(loc, state,
"the \"binding\" qualifier only applies to uniforms");
return false;
@@ -2056,10 +2056,11 @@ validate_binding_qualifier(struct
_mesa_glsl_parse_state *state,
}
const struct gl_context *const ctx = state->ctx;
- unsigned elements = var->type->is_array() ? var->type->length : 1;
+ unsigned elements = type->is_array() ? type->length : 1;
unsigned max_index = qual->binding + elements - 1;
+ const glsl_type *base_type = type->without_array();
- if (var->type->is_interface()) {
+ if (base_type->is_interface()) {
/* UBOs. From page 60 of the GLSL 4.20 specification:
* "If the binding point for any uniform block instance is less than
zero,
* or greater than or equal to the implementation-dependent maximum
@@ -2077,8 +2078,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
ctx->Const.MaxUniformBufferBindings);
return false;
}
- } else if (var->type->is_sampler() ||
- (var->type->is_array() &&
var->type->fields.array->is_sampler())) {
+ } else if (base_type->is_sampler()) {
/* Samplers. From page 63 of the GLSL 4.20 specification:
* "If the binding is less than zero, or greater than or equal to the
* implementation-dependent maximum supported number of units, a
@@ -2095,7 +2095,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
return false;
}
- } else if (var->type->contains_atomic()) {
+ } else if (base_type->contains_atomic()) {
assert(ctx->Const.MaxAtomicBufferBindings <=
MAX_COMBINED_ATOMIC_BUFFERS);
if (unsigned(qual->binding) >= ctx->Const.MaxAtomicBufferBindings) {
_mesa_glsl_error(loc, state, "layout(binding = %d) exceeds the "
@@ -2651,7 +2651,7 @@ apply_type_qualifier_to_variable(const struct
ast_type_qualifier *qual,
}
if (qual->flags.q.explicit_binding &&
- validate_binding_qualifier(state, loc, var, qual)) {
+ validate_binding_qualifier(state, loc, var->type, qual)) {
var->data.explicit_binding = true;
var->data.binding = qual->binding;
}
@@ -5752,6 +5752,8 @@ ast_interface_block::hir(exec_list *instructions,
num_variables,
packing,
this->block_name);
+ if (this->layout.flags.q.explicit_binding)
+ validate_binding_qualifier(state, &loc, block_type, &this->layout);
if (!state->symbols->add_interface(block_type->name, block_type, var_mode))
{
YYLTYPE loc = this->get_location();
@@ -5849,6 +5851,10 @@ ast_interface_block::hir(exec_list *instructions,
"not allowed");
}
+ if (this->layout.flags.q.explicit_binding)
+validate_binding_qualifier(state, &loc, block_array_type,
+ &this->layout);
+
var = new(state) ir_variable(block_array_type,
this->instance_name,
var_mode);
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/16] glsl: add helper for calculating size of AoA
---
src/glsl/glsl_types.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
index 93a1d25..87104cb 100644
--- a/src/glsl/glsl_types.h
+++ b/src/glsl/glsl_types.h
@@ -537,6 +537,25 @@ struct glsl_type {
}
/**
+* Return the total number of elements in an array including the elements
+* in arrays of arrays.
+*/
+ unsigned arrays_of_arrays_size() const
+ {
+ if (!is_array())
+ return -1;
+
+ unsigned size = length;
+ const glsl_type *base_type = fields.array;
+
+ while (base_type->is_array()) {
+ size = size * base_type->length;
+ base_type = base_type->fields.array;
+ }
+ return size;
+ }
+
+ /**
* Return the amount of atomic counter storage required for a type.
*/
unsigned atomic_size() const
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/16] glsl: dont allow gl_PerVertex to be redeclared as an array of arrays
V2: move single dimensionial array detection into a helper
Signed-off-by: Timothy Arceri
---
src/glsl/ast.h | 8
src/glsl/ast_to_hir.cpp | 3 ++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index ef74e51..3f67907 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -328,6 +328,14 @@ public:
array_dimensions.push_tail(&dim->link);
}
+ bool is_single_dimension()
+ {
+ return (this->is_unsized_array && this->array_dimensions.is_empty()) ||
+ (!this->is_unsized_array &&
+ this->array_dimensions.tail_pred->prev != NULL &&
+ this->array_dimensions.tail_pred->prev->is_head_sentinel());
+ }
+
virtual void print(void) const;
/* If true, this means that the array has an unsized outermost dimension. */
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index de6a86d..1e3686c 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -5670,7 +5670,8 @@ ast_interface_block::hir(exec_list *instructions,
_mesa_shader_stage_to_string(state->stage));
}
if (this->instance_name == NULL ||
- strcmp(this->instance_name, "gl_in") != 0 ||
this->array_specifier == NULL) {
+ strcmp(this->instance_name, "gl_in") != 0 ||
this->array_specifier == NULL ||
+ !this->array_specifier->is_single_dimension()) {
_mesa_glsl_error(&loc, state,
"gl_PerVertex input must be redeclared as "
"gl_in[]");
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 13/16] glsl: validate binding qualifier for AoA
--- src/glsl/ast_to_hir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 341f9c4..0d0364b 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -2056,7 +2056,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state, } const struct gl_context *const ctx = state->ctx; - unsigned elements = type->is_array() ? type->length : 1; + unsigned elements = type->is_array() ? type->arrays_of_arrays_size() : 1; unsigned max_index = qual->binding + elements - 1; const glsl_type *base_type = type->without_array(); -- 2.4.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/16] glsl: clean-up link uniform code
---
src/glsl/link_uniforms.cpp | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 11ae06f..2d50b9b 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -71,6 +71,7 @@ void
program_resource_visitor::process(ir_variable *var)
{
const glsl_type *t = var->type;
+ const glsl_type *t_without_array = var->type->without_array();
const bool row_major =
var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
@@ -140,12 +141,8 @@ program_resource_visitor::process(ir_variable *var)
char *name = ralloc_strdup(NULL, var->name);
recursion(var->type, &name, strlen(name), row_major, NULL, false);
ralloc_free(name);
- } else if (t->is_interface()) {
- char *name = ralloc_strdup(NULL, var->type->name);
- recursion(var->type, &name, strlen(name), row_major, NULL, false);
- ralloc_free(name);
- } else if (t->is_array() && t->fields.array->is_interface()) {
- char *name = ralloc_strdup(NULL, var->type->fields.array->name);
+ } else if (t_without_array->is_interface()) {
+ char *name = ralloc_strdup(NULL, t_without_array->name);
recursion(var->type, &name, strlen(name), row_major, NULL, false);
ralloc_free(name);
} else {
@@ -216,8 +213,8 @@ program_resource_visitor::recursion(const glsl_type *t,
char **name,
(*name)[name_length] = '\0';
this->leave_record(t, *name, row_major);
}
- } else if (t->is_array() && (t->fields.array->is_record()
-|| t->fields.array->is_interface())) {
+ } else if (t->without_array()->is_record()
+ || t->without_array()->is_interface()) {
if (record_type == NULL && t->fields.array->is_record())
record_type = t->fields.array;
@@ -778,8 +775,7 @@ link_update_uniform_buffer_variables(struct gl_shader
*shader)
if (var->type->is_record()) {
sentinel = '.';
- } else if (var->type->is_array()
- && var->type->fields.array->is_record()) {
+ } else if (var->type->without_array()->is_record()) {
sentinel = '[';
}
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 16/16] glsl: Allow arrays of arrays in GLSL ES 3.10 and GLSL 4.30
V2: add missing 's' to the extension name in error messages
and add decimal place in version string
---
src/glsl/ast_to_hir.cpp | 13 -
src/glsl/glsl_parser.yy | 22 ++
src/glsl/glsl_parser_extras.h | 5 +
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 8154c1e..fe169b3 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1939,12 +1939,15 @@ process_array_type(YYLTYPE *loc, const glsl_type *base,
*
* "Only one-dimensional arrays may be declared."
*/
- if (!state->ARB_arrays_of_arrays_enable) {
+ if (!state->has_arrays_of_arrays()) {
+const char *const requirement = state->es_shader
+ ? "GLSL ES 3.10"
+ : "GL_ARB_arrays_of_arrays or GLSL 4.30";
_mesa_glsl_error(loc, state,
- "invalid array of `%s'"
- "GL_ARB_arrays_of_arrays "
- "required for defining arrays of arrays",
- base->name);
+ "invalid array of `%s' "
+ "%s required for defining arrays of arrays",
+ base->name, requirement);
+
return glsl_type::error_type;
}
}
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 1f08893..c480689 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -1856,10 +1856,13 @@ array_specifier:
void *ctx = state;
$$ = $1;
- if (!state->ARB_arrays_of_arrays_enable) {
- _mesa_glsl_error(& @1, state,
- "GL_ARB_arrays_of_arrays "
- "required for defining arrays of arrays");
+ if (!state->has_arrays_of_arrays()) {
+ const char *const requirement = state->es_shader
+? "GLSL ES 3.10"
+: "GL_ARB_arrays_of_arrays or GLSL 4.30";
+ _mesa_glsl_error(& @1, state,
+ "%s required for defining arrays of arrays.",
+ requirement);
}
$$->add_dimension(new(ctx) ast_expression(ast_unsized_array_dim, NULL,
NULL, NULL));
@@ -1868,10 +1871,13 @@ array_specifier:
{
$$ = $1;
- if (!state->ARB_arrays_of_arrays_enable) {
- _mesa_glsl_error(& @1, state,
- "GL_ARB_arrays_of_arrays "
- "required for defining arrays of arrays");
+ if (!state->has_arrays_of_arrays()) {
+ const char *const requirement = state->es_shader
+? "GLSL ES 3.10"
+: "GL_ARB_arrays_of_arrays or GLSL 4.30";
+ _mesa_glsl_error(& @1, state,
+ "%s required for defining arrays of arrays.",
+ requirement);
}
$$->add_dimension($3);
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index 02ddbbd..ba760fe 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -190,6 +190,11 @@ struct _mesa_glsl_parse_state {
return true;
}
+ bool has_arrays_of_arrays() const
+ {
+ return ARB_arrays_of_arrays_enable || is_version(430, 310);
+ }
+
bool has_atomic_counters() const
{
return ARB_shader_atomic_counters_enable || is_version(420, 310);
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 15/16] glsl: remove dead code in a single pass
Currently only one ir assignment is removed for each var in a single
dead code optimisation pass. This means if a var has more than one
assignment, then it requires all the glsl optimisations to be run again
for each additional assignment to be removed.
Another pass is also required to remove the variable itself.
With this change all assignments and the variable are removed in a single
pass.
Some of the arrays of arrays conformance tests that were looping
through 8 dimensions ended up with a var with hundreds of assignments.
This change helps ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1
go from around 3 min 20 sec -> 2 min
ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 went from
around 9 min 20 sec to 7 min 30 sec
I had difficulty getting the public shader-db to give a constent result
with or without this change but the results seemed unchanged at between
15-20 seconds.
Thomas Helland measured change with shader-db on his machine from
approx 117 secs to 112 secs.
V2: Add assert to be sure references are counted before assignments.
Tested-By: Thomas Helland
---
src/glsl/ir_variable_refcount.cpp | 28 +---
src/glsl/ir_variable_refcount.h | 13 -
src/glsl/opt_dead_code.cpp| 30 +-
src/glsl/opt_tree_grafting.cpp| 2 --
4 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/src/glsl/ir_variable_refcount.cpp
b/src/glsl/ir_variable_refcount.cpp
index e4d825c..4ca492e 100644
--- a/src/glsl/ir_variable_refcount.cpp
+++ b/src/glsl/ir_variable_refcount.cpp
@@ -46,6 +46,16 @@ static void
free_entry(struct hash_entry *entry)
{
ir_variable_refcount_entry *ivre = (ir_variable_refcount_entry *)
entry->data;
+
+ /* Free assignment list */
+ while (!ivre->assign_list.is_empty()) {
+ struct assignment_entry *assignment_entry =
+ exec_node_data(struct assignment_entry,
+ivre->assign_list.head, link);
+ assignment_entry->link.remove();
+ free(assignment_entry);
+ }
+
delete ivre;
}
@@ -59,7 +69,6 @@ ir_variable_refcount_visitor::~ir_variable_refcount_visitor()
ir_variable_refcount_entry::ir_variable_refcount_entry(ir_variable *var)
{
this->var = var;
- assign = NULL;
assigned_count = 0;
declaration = false;
referenced_count = 0;
@@ -125,8 +134,21 @@ ir_variable_refcount_visitor::visit_leave(ir_assignment
*ir)
entry = this->get_variable_entry(ir->lhs->variable_referenced());
if (entry) {
entry->assigned_count++;
- if (entry->assign == NULL)
-entry->assign = ir;
+
+ /**
+ * Build a list for dead code optimisation. Dont add assingment if it
+ * was declared out of scope (outside the instruction stream). Also dont
+ * bother adding any more to the list if there are more references then
+ * assignments as this means the variable is used and won't be optimised
+ * out.
+ */
+ assert(entry->referenced_count >= entry->assigned_count);
+ if (entry->referenced_count == entry->assigned_count) {
+ struct assignment_entry *assignment_entry =
+(struct assignment_entry *)calloc(1, sizeof(*assignment_entry));
+ assignment_entry->assign = ir;
+ entry->assign_list.push_head(&assignment_entry->link);
+ }
}
return visit_continue;
diff --git a/src/glsl/ir_variable_refcount.h b/src/glsl/ir_variable_refcount.h
index c15e8110..5c74c31 100644
--- a/src/glsl/ir_variable_refcount.h
+++ b/src/glsl/ir_variable_refcount.h
@@ -33,13 +33,24 @@
#include "ir_visitor.h"
#include "glsl_types.h"
+struct assignment_entry {
+ exec_node link;
+ ir_assignment *assign;
+};
+
class ir_variable_refcount_entry
{
public:
ir_variable_refcount_entry(ir_variable *var);
ir_variable *var; /* The key: the variable's pointer. */
- ir_assignment *assign; /* An assignment to the variable, if any */
+
+ /**
+* List of assignments to the variable, if any.
+* This is intended to be used for dead code optimisation and may
+* not be a complete list.
+*/
+ exec_list assign_list;
/** Number of times the variable is referenced, including assignments. */
unsigned referenced_count;
diff --git a/src/glsl/opt_dead_code.cpp b/src/glsl/opt_dead_code.cpp
index f45bf5d..6d1f675 100644
--- a/src/glsl/opt_dead_code.cpp
+++ b/src/glsl/opt_dead_code.cpp
@@ -75,22 +75,34 @@ do_dead_code(exec_list *instructions, bool
uniform_locations_assigned)
|| !entry->declaration)
continue;
- if (entry->assign) {
-/* Remove a single dead assignment to the variable we found.
+ if (!entry->assign_list.is_empty()) {
+/* Remove all the dead assignments to the variable we found.
* Don't do so if it's a shader or function output, though.
*/
if (entry->var->data.mode != ir_var_function_out &&
entry->var->data.mode != ir_var_function_inout &&
en
[Mesa-dev] [PATCH 14/16] glsl: allow AoA to be sized by initializer or constructor
From Section 4.1.9 of the GLSL ES 3.10 spec:
"Arrays are sized either at compile-time or at run-time.
To size an array at compile-time, either the size
must be specified within the brackets as above or
must be inferred from the type of the initializer."
---
src/glsl/ast.h | 21 +++-
src/glsl/ast_array_index.cpp | 7 ++--
src/glsl/ast_function.cpp| 33 +-
src/glsl/ast_to_hir.cpp | 79 ++--
src/glsl/glsl_parser.yy | 11 +++---
5 files changed, 106 insertions(+), 45 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 3f67907..0f9dbf9 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -181,6 +181,7 @@ enum ast_operators {
ast_post_dec,
ast_field_selection,
ast_array_index,
+ ast_unsized_array_dim,
ast_function_call,
@@ -308,16 +309,7 @@ private:
class ast_array_specifier : public ast_node {
public:
- /** Unsized array specifier ([]) */
- explicit ast_array_specifier(const struct YYLTYPE &locp)
- : is_unsized_array(true)
- {
- set_location(locp);
- }
-
- /** Sized array specifier ([dim]) */
ast_array_specifier(const struct YYLTYPE &locp, ast_expression *dim)
- : is_unsized_array(false)
{
set_location(locp);
array_dimensions.push_tail(&dim->link);
@@ -330,19 +322,14 @@ public:
bool is_single_dimension()
{
- return (this->is_unsized_array && this->array_dimensions.is_empty()) ||
- (!this->is_unsized_array &&
- this->array_dimensions.tail_pred->prev != NULL &&
- this->array_dimensions.tail_pred->prev->is_head_sentinel());
+ return this->array_dimensions.tail_pred->prev != NULL &&
+ this->array_dimensions.tail_pred->prev->is_head_sentinel();
}
virtual void print(void) const;
- /* If true, this means that the array has an unsized outermost dimension. */
- bool is_unsized_array;
-
/* This list contains objects of type ast_node containing the
-* sized dimensions only, in outermost-to-innermost order.
+* array dimensions in outermost-to-innermost order.
*/
exec_list array_dimensions;
};
diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
index 2c79002..49090e9 100644
--- a/src/glsl/ast_array_index.cpp
+++ b/src/glsl/ast_array_index.cpp
@@ -28,13 +28,10 @@
void
ast_array_specifier::print(void) const
{
- if (this->is_unsized_array) {
- printf("[ ] ");
- }
-
foreach_list_typed (ast_node, array_dimension, link,
&this->array_dimensions) {
printf("[ ");
- array_dimension->print();
+ if (((ast_expression*)array_dimension)->oper != ast_unsized_array_dim)
+ array_dimension->print();
printf("] ");
}
}
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 92e26bf..0906040 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -870,6 +870,7 @@ process_array_constructor(exec_list *instructions,
}
bool all_parameters_are_constant = true;
+ const glsl_type *element_type = constructor_type->fields.array;
/* Type cast each parameter and, if possible, fold constants. */
foreach_in_list_safe(ir_rvalue, ir, &actual_parameters) {
@@ -896,12 +897,34 @@ process_array_constructor(exec_list *instructions,
}
}
- if (result->type != constructor_type->fields.array) {
+ if (constructor_type->fields.array->is_unsized_array()) {
+ /* As the inner parameters of the constructor are created without
+ * knowledge of each other we need to check to make sure unsized
+ * parameters of unsized constructors all end up with the same size.
+ *
+ * e.g we make sure to fail for a constructor like this:
+ * vec4[][] a = vec4[][](vec4[](vec4(0.0), vec4(1.0)),
+ * vec4[](vec4(0.0), vec4(1.0), vec4(1.0)),
+ * vec4[](vec4(0.0), vec4(1.0)));
+ */
+ if (element_type->is_unsized_array()) {
+ /* This is the first parameter so just get the type */
+element_type = result->type;
+ } else if (element_type != result->type) {
+_mesa_glsl_error(loc, state, "type error in array constructor: "
+ "expected: %s, found %s",
+ element_type->name,
+ result->type->name);
+return ir_rvalue::error_value(ctx);
+ }
+ } else if (result->type != constructor_type->fields.array) {
_mesa_glsl_error(loc, state, "type error in array constructor: "
"expected: %s, found %s",
constructor_type->fields.array->name,
result->type->name);
return ir_rvalue::error_value(ctx);
+ } else {
+ element_type = result->type;
}
/* Attempt to convert the parameter to a constant valu
Re: [Mesa-dev] [PATCH 11/16] glsl: fix binding validation for interface blocks
On Sat, 2015-07-18 at 11:25 +1000, Timothy Arceri wrote:
> ---
> src/glsl/ast_to_hir.cpp | 22 ++
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 1e3686c..341f9c4 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2041,10 +2041,10 @@ validate_matrix_layout_for_type(struct
> _mesa_glsl_parse_state *state,
> static bool
> validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
> YYLTYPE *loc,
> - ir_variable *var,
> + const glsl_type *type,
> const ast_type_qualifier *qual)
> {
> - if (var->data.mode != ir_var_uniform) {
> + if (!qual->flags.q.uniform) {
I forgot to rebase before sending this out this is line is now:
if (!qual->flags.q.uniform && !qual->flags.q.buffer) {
>_mesa_glsl_error(loc, state,
> "the \"binding\" qualifier only applies to
> uniforms");
>return false;
> @@ -2056,10 +2056,11 @@ validate_binding_qualifier(struct
> _mesa_glsl_parse_state *state,
> }
>
> const struct gl_context *const ctx = state->ctx;
> - unsigned elements = var->type->is_array() ? var->type->length : 1;
> + unsigned elements = type->is_array() ? type->length : 1;
> unsigned max_index = qual->binding + elements - 1;
> + const glsl_type *base_type = type->without_array();
>
> - if (var->type->is_interface()) {
> + if (base_type->is_interface()) {
>/* UBOs. From page 60 of the GLSL 4.20 specification:
> * "If the binding point for any uniform block instance is less than
> zero,
> * or greater than or equal to the implementation-dependent maximum
> @@ -2077,8 +2078,7 @@ validate_binding_qualifier(struct
> _mesa_glsl_parse_state *state,
>ctx->Const.MaxUniformBufferBindings);
> return false;
>}
> - } else if (var->type->is_sampler() ||
> - (var->type->is_array() && var->type->fields.array
> ->is_sampler())) {
> + } else if (base_type->is_sampler()) {
>/* Samplers. From page 63 of the GLSL 4.20 specification:
> * "If the binding is less than zero, or greater than or equal to the
> * implementation-dependent maximum supported number of units, a
> @@ -2095,7 +2095,7 @@ validate_binding_qualifier(struct
> _mesa_glsl_parse_state *state,
>
> return false;
>}
> - } else if (var->type->contains_atomic()) {
> + } else if (base_type->contains_atomic()) {
>assert(ctx->Const.MaxAtomicBufferBindings <=
> MAX_COMBINED_ATOMIC_BUFFERS);
>if (unsigned(qual->binding) >= ctx->Const.MaxAtomicBufferBindings) {
> _mesa_glsl_error(loc, state, "layout(binding = %d) exceeds the "
> @@ -2651,7 +2651,7 @@ apply_type_qualifier_to_variable(const struct
> ast_type_qualifier *qual,
> }
>
> if (qual->flags.q.explicit_binding &&
> - validate_binding_qualifier(state, loc, var, qual)) {
> + validate_binding_qualifier(state, loc, var->type, qual)) {
>var->data.explicit_binding = true;
>var->data.binding = qual->binding;
> }
> @@ -5752,6 +5752,8 @@ ast_interface_block::hir(exec_list *instructions,
> num_variables,
> packing,
> this->block_name);
> + if (this->layout.flags.q.explicit_binding)
> + validate_binding_qualifier(state, &loc, block_type, &this->layout);
>
> if (!state->symbols->add_interface(block_type->name, block_type,
> var_mode)) {
>YYLTYPE loc = this->get_location();
> @@ -5849,6 +5851,10 @@ ast_interface_block::hir(exec_list *instructions,
> "not allowed");
> }
>
> + if (this->layout.flags.q.explicit_binding)
> +validate_binding_qualifier(state, &loc, block_array_type,
> + &this->layout);
> +
> var = new(state) ir_variable(block_array_type,
>this->instance_name,
>var_mode);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/16] glsl: fix binding validation for interface blocks
V2: rebase on SSBO changes
---
src/glsl/ast_to_hir.cpp | 27 +--
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 1eafdc2..694f2fe 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2048,10 +2048,10 @@ validate_matrix_layout_for_type(struct
_mesa_glsl_parse_state *state,
static bool
validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
YYLTYPE *loc,
- ir_variable *var,
+ const glsl_type *type,
const ast_type_qualifier *qual)
{
- if (var->data.mode != ir_var_uniform && var->data.mode !=
ir_var_shader_storage) {
+ if (!qual->flags.q.uniform && !qual->flags.q.buffer) {
_mesa_glsl_error(loc, state,
"the \"binding\" qualifier only applies to uniforms and
"
"shader storage buffer objects");
@@ -2064,10 +2064,11 @@ validate_binding_qualifier(struct
_mesa_glsl_parse_state *state,
}
const struct gl_context *const ctx = state->ctx;
- unsigned elements = var->type->is_array() ? var->type->length : 1;
+ unsigned elements = type->is_array() ? type->length : 1;
unsigned max_index = qual->binding + elements - 1;
+ const glsl_type *base_type = type->without_array();
- if (var->type->is_interface()) {
+ if (base_type->is_interface()) {
/* UBOs. From page 60 of the GLSL 4.20 specification:
* "If the binding point for any uniform block instance is less than
zero,
* or greater than or equal to the implementation-dependent maximum
@@ -2078,7 +2079,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
*
* The implementation-dependent maximum is
GL_MAX_UNIFORM_BUFFER_BINDINGS.
*/
- if (var->data.mode == ir_var_uniform &&
+ if (qual->flags.q.uniform &&
max_index >= ctx->Const.MaxUniformBufferBindings) {
_mesa_glsl_error(loc, state, "layout(binding = %d) for %d UBOs
exceeds "
"the maximum number of UBO binding points (%d)",
@@ -2086,6 +2087,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
ctx->Const.MaxUniformBufferBindings);
return false;
}
+
/* SSBOs. From page 67 of the GLSL 4.30 specification:
* "If the binding point for any uniform or shader storage block instance
* is less than zero, or greater than or equal to the
@@ -2095,7 +2097,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
* N, all elements of the array from binding through binding + N – 1
must
* be within this range."
*/
- if (var->data.mode == ir_var_shader_storage &&
+ if (qual->flags.q.buffer &&
max_index >= ctx->Const.MaxShaderStorageBufferBindings) {
_mesa_glsl_error(loc, state, "layout(binding = %d) for %d SSBOs
exceeds "
"the maximum number of SSBO binding points (%d)",
@@ -2103,8 +2105,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
ctx->Const.MaxShaderStorageBufferBindings);
return false;
}
- } else if (var->type->is_sampler() ||
- (var->type->is_array() &&
var->type->fields.array->is_sampler())) {
+ } else if (base_type->is_sampler()) {
/* Samplers. From page 63 of the GLSL 4.20 specification:
* "If the binding is less than zero, or greater than or equal to the
* implementation-dependent maximum supported number of units, a
@@ -2121,7 +2122,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state
*state,
return false;
}
- } else if (var->type->contains_atomic()) {
+ } else if (base_type->contains_atomic()) {
assert(ctx->Const.MaxAtomicBufferBindings <=
MAX_COMBINED_ATOMIC_BUFFERS);
if (unsigned(qual->binding) >= ctx->Const.MaxAtomicBufferBindings) {
_mesa_glsl_error(loc, state, "layout(binding = %d) exceeds the "
@@ -2679,7 +2680,7 @@ apply_type_qualifier_to_variable(const struct
ast_type_qualifier *qual,
}
if (qual->flags.q.explicit_binding &&
- validate_binding_qualifier(state, loc, var, qual)) {
+ validate_binding_qualifier(state, loc, var->type, qual)) {
var->data.explicit_binding = true;
var->data.binding = qual->binding;
}
@@ -5806,6 +5807,8 @@ ast_interface_block::hir(exec_list *instructions,
num_variables,
packing,
this->block_name);
+ if (this->layout.flags.q.explicit_binding)
+ validate_binding_qualifier(state, &loc, block_type, &this->layout);
if (!state->symbols->add_interface(block_type->name, block_type, var_mode))
{
YYLTYPE loc = this->get_location();
@@ -5903,6
Re: [Mesa-dev] [PATCH 02/16] mesa: fix incorrect comment
Yeah your right but its also used by _mesa_program_resource_prop I can move the comment but is it really adding any value anyway? On Fri, 2015-07-17 at 21:31 -0400, Ilia Mirkin wrote: > The comment is misplaced, but not entirely wrong. It should be on the > program_resource_location helper. That helper is in fact used by the > other 2 functions > > On Fri, Jul 17, 2015 at 9:25 PM, Timothy Arceri > wrote: > > --- > > src/mesa/main/shader_query.cpp | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/src/mesa/main/shader_query.cpp > > b/src/mesa/main/shader_query.cpp > > index a6246a3..0473c2e 100644 > > --- a/src/mesa/main/shader_query.cpp > > +++ b/src/mesa/main/shader_query.cpp > > @@ -787,8 +787,6 @@ program_resource_location(struct gl_shader_program > > *shProg, > > > > /** > > * Function implements following location queries: > > - *glGetAttribLocation > > - *glGetFragDataLocation > > *glGetUniformLocation > > */ > > GLint > > -- > > 2.4.3 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] V2 ARB_arrays_of_arrays GLSL ES
Hi all, The restrictions in ES make the extension easier to implement so I thought I'd try get this stuff reviewed an committed before finishing up the full extension. The bits that I'm still working on for the desktop version are AoA inputs outputs, and interface blocks. The only thing I know is definatly missing in this series for ES is support for indirect indexing of samplers, but that didn't seem like something that should hold up the series. Once the SSBO series lands (with a patch that restricts unsized arrays) then all the AoA ES conformance tests will pass. There are already a bunch of piglit tests in git but I'm also waiting on some patches to be reviewed here: http://lists.freedesktop.org/archives/piglit/2015-July/016442.html If useful the series is in the 'gles7' branch of the repo here: https://github.com/tarceri/Mesa_arrays_of_arrays.git Changes in v2: - Reworked how uniform AoA are handled which simplified things greatly. - Transform feedback and program interface query now working with AoA. - Speed up glsl optimisation pass via improvement to dead code elimination. - Some smaller reviewed patches pushed. Thanks, Tim ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/16] glsl: clean-up link uniform code
On Tue, 2015-07-21 at 10:03 +0200, Samuel Iglesias Gonsálvez wrote:
> On 18/07/15 03:25, Timothy Arceri wrote:
> > ---
> > src/glsl/link_uniforms.cpp | 16 ++--
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> > index 11ae06f..2d50b9b 100644
> > --- a/src/glsl/link_uniforms.cpp
> > +++ b/src/glsl/link_uniforms.cpp
> > @@ -71,6 +71,7 @@ void
> > program_resource_visitor::process(ir_variable *var)
> > {
> > const glsl_type *t = var->type;
> > + const glsl_type *t_without_array = var->type->without_array();
> > const bool row_major =
> >var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
> >
> > @@ -140,12 +141,8 @@ program_resource_visitor::process(ir_variable *var)
> >char *name = ralloc_strdup(NULL, var->name);
> >recursion(var->type, &name, strlen(name), row_major, NULL, false);
> >ralloc_free(name);
> > - } else if (t->is_interface()) {
> > - char *name = ralloc_strdup(NULL, var->type->name);
> > - recursion(var->type, &name, strlen(name), row_major, NULL, false);
> > - ralloc_free(name);
> > - } else if (t->is_array() && t->fields.array->is_interface()) {
> > - char *name = ralloc_strdup(NULL, var->type->fields.array->name);
> > + } else if (t_without_array->is_interface()) {
> > + char *name = ralloc_strdup(NULL, t_without_array->name);
> >recursion(var->type, &name, strlen(name), row_major, NULL, false);
> >ralloc_free(name);
> > } else {
> > @@ -216,8 +213,8 @@ program_resource_visitor::recursion(const glsl_type
> > *t, char **name,
> > (*name)[name_length] = '\0';
> > this->leave_record(t, *name, row_major);
> >}
> > - } else if (t->is_array() && (t->fields.array->is_record()
> > -|| t->fields.array->is_interface())) {
> > + } else if (t->without_array()->is_record()
> > + || t->without_array()->is_interface()) {
>
> I have my doubts about this change. It is not wrong per se but I prefer
> to keep the context the original code gives to the reader.
>
> With your change, readers might not see that we are checking an array of
> interfaces/records and they could ask themselves why this code is not
> merged to previous t->is_interface() or t->is_record() cases.
>
> I would do only that change if we are planning to merge this code to the
> non-array case.
>
> >if (record_type == NULL && t->fields.array->is_record())
> > record_type = t->fields.array;
> >
> > @@ -778,8 +775,7 @@ link_update_uniform_buffer_variables(struct gl_shader
> > *shader)
> >
> >if (var->type->is_record()) {
> > sentinel = '.';
> > - } else if (var->type->is_array()
> > - && var->type->fields.array->is_record()) {
> > + } else if (var->type->without_array()->is_record()) {
>
> Same doubt as before.
>
> However, I think it is a matter of taste. What do you think?
I did have the same concern as you initially. My commit message is a little
misleading the change also adds support for AoA to structs and interfaces too.
I split this change out of a different patch and forgot to mention it (I'll
update the commit message).
I guess it's probably a good idea to add some comments to the code to make the
differences more clear, I'll send a new version with this change.
On the positive side if someone did try combine these then piglit tests would
fail pretty quickly.
Thanks for the reviews.
Tim
>
> Sam
>
> > sentinel = '[';
> >}
> >
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC] AoA atomic counter follow-up fixes
Hi Guys, These patches fix up AoA support for atomic counters following my change in approach to uniforms in V2 of my AoA GLES series. However I wanted to get some feedback mainly on the first patch as the end result is the whole AoA is marked as active which doesnt happen on Nvidia drivers. I guess my question is do you think this patch is ok as a first pass at just getting this stuff up and running or should I spend a bunch of time writting a pass that splits the outer arrays into separate variables to make them easier to handle separatly. At this stage in the glsl ir life cycle I don't know if its worth the extra effort. Thanks, Tim ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 17/16] glsl: add AoA support for atmoic counters
---
src/glsl/link_atomics.cpp | 73 +++
1 file changed, 55 insertions(+), 18 deletions(-)
diff --git a/src/glsl/link_atomics.cpp b/src/glsl/link_atomics.cpp
index 100d03c..21f9af7 100644
--- a/src/glsl/link_atomics.cpp
+++ b/src/glsl/link_atomics.cpp
@@ -95,6 +95,56 @@ namespace {
y->data.atomic.offset < x->data.atomic.offset +
x->type->atomic_size()));
}
+ void
+ process_atomic_variable(const glsl_type *t, struct gl_shader_program *prog,
+ char **name, size_t name_length, ir_variable *var,
+ active_atomic_buffer *const buffers,
+ unsigned *num_buffers, int *offset,
+ const unsigned shader_stage)
+ {
+ /* FIXME: Arrays of arrays get counted separately. For example:
+ * x1[3][3][2] = 9 counters
+ * x2[3][2]= 3 counters
+ * x3[2] = 1 counter
+ *
+ * However this code marks all the counters as active even when they
+ * might not be used.
+ */
+ if (t->is_array() && t->fields.array->is_array()) {
+ for (unsigned i = 0; i < t->length; i++) {
+ size_t new_length = name_length;
+
+ /* Append the subscript to the current variable name */
+ ralloc_asprintf_rewrite_tail(name, &new_length, "[%u]", i);
+
+process_atomic_variable(t->fields.array, prog, name, new_length,
+var, buffers, num_buffers, offset,
+shader_stage);
+ }
+ } else {
+ unsigned id = 0;
+ bool found = prog->UniformHash->get(id, *name);
+ assert(found);
+ (void) found;
+ active_atomic_buffer *buf = &buffers[var->data.binding];
+ gl_uniform_storage *const storage = &prog->UniformStorage[id];
+
+ /* If this is the first time the buffer is used, increment
+ * the counter of buffers used.
+ */
+ if (buf->size == 0)
+(*num_buffers)++;
+
+ buf->push_back(id, var);
+
+ buf->stage_references[shader_stage]++;
+ buf->size = MAX2(buf->size, *offset + t->atomic_size());
+
+ storage->offset = *offset;
+ *offset += t->atomic_size();
+ }
+ }
+
active_atomic_buffer *
find_active_atomic_counters(struct gl_context *ctx,
struct gl_shader_program *prog,
@@ -114,23 +164,11 @@ namespace {
ir_variable *var = node->as_variable();
if (var && var->type->contains_atomic()) {
- unsigned id = 0;
- bool found = prog->UniformHash->get(id, var->name);
- assert(found);
- (void) found;
- active_atomic_buffer *buf = &buffers[var->data.binding];
-
- /* If this is the first time the buffer is used, increment
-* the counter of buffers used.
-*/
- if (buf->size == 0)
- (*num_buffers)++;
-
- buf->push_back(id, var);
-
- buf->stage_references[i]++;
- buf->size = MAX2(buf->size, var->data.atomic.offset +
-var->type->atomic_size());
+ char *name = ralloc_strdup(NULL, var->name);
+ int offset = var->data.atomic.offset;
+ process_atomic_variable(var->type, prog, &name, strlen(name),
+ var, buffers, num_buffers, &offset, i);
+ ralloc_free(name);
}
}
}
@@ -205,7 +243,6 @@ link_assign_atomic_counter_resources(struct gl_context *ctx,
var->data.binding = i;
storage->atomic_buffer_index = i;
- storage->offset = var->data.atomic.offset;
storage->array_stride = (var->type->is_array() ?
var->type->without_array()->atomic_size() :
0);
}
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 18/16] nir: wrapper for glsl_type arrays_of_arrays_size()
---
src/glsl/nir/nir_types.cpp | 6 ++
src/glsl/nir/nir_types.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/src/glsl/nir/nir_types.cpp b/src/glsl/nir/nir_types.cpp
index 62176f5..06f5b0a 100644
--- a/src/glsl/nir/nir_types.cpp
+++ b/src/glsl/nir/nir_types.cpp
@@ -106,6 +106,12 @@ glsl_get_length(const struct glsl_type *type)
return type->is_matrix() ? type->matrix_columns : type->length;
}
+unsigned
+glsl_get_aoa_size(const struct glsl_type *type)
+{
+ return type->arrays_of_arrays_size();
+}
+
const char *
glsl_get_struct_elem_name(const struct glsl_type *type, unsigned index)
{
diff --git a/src/glsl/nir/nir_types.h b/src/glsl/nir/nir_types.h
index 276d4ad..3f75e46 100644
--- a/src/glsl/nir/nir_types.h
+++ b/src/glsl/nir/nir_types.h
@@ -59,6 +59,8 @@ unsigned glsl_get_matrix_columns(const struct glsl_type
*type);
unsigned glsl_get_length(const struct glsl_type *type);
+unsigned glsl_get_aoa_size(const struct glsl_type *type);
+
const char *glsl_get_struct_elem_name(const struct glsl_type *type,
unsigned index);
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 19/16] nir: add atomic lowering support for AoA
---
src/glsl/nir/nir_lower_atomics.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/src/glsl/nir/nir_lower_atomics.c b/src/glsl/nir/nir_lower_atomics.c
index ce3615a..e5ec008 100644
--- a/src/glsl/nir/nir_lower_atomics.c
+++ b/src/glsl/nir/nir_lower_atomics.c
@@ -72,15 +72,17 @@ lower_instr(nir_intrinsic_instr *instr, nir_function_impl
*impl)
nir_ssa_def *offset_def = &offset_const->def;
- if (instr->variables[0]->deref.child != NULL) {
- assert(instr->variables[0]->deref.child->deref_type ==
- nir_deref_type_array);
- nir_deref_array *deref_array =
- nir_deref_as_array(instr->variables[0]->deref.child);
- assert(deref_array->deref.child == NULL);
-
- offset_const->value.u[0] +=
- deref_array->base_offset * ATOMIC_COUNTER_SIZE;
+ nir_deref *tail = &instr->variables[0]->deref;
+ while (tail->child != NULL) {
+ assert(tail->child->deref_type == nir_deref_type_array);
+ nir_deref_array *deref_array = nir_deref_as_array(tail->child);
+ tail = tail->child;
+
+ unsigned child_array_elements = tail != NULL ?
+ glsl_get_aoa_size(tail->type) : 1;
+
+ offset_const->value.u[0] += deref_array->base_offset *
+ child_array_elements * ATOMIC_COUNTER_SIZE;
if (deref_array->deref_array_type == nir_deref_array_type_indirect) {
nir_load_const_instr *atomic_counter_size =
--
2.4.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 16/20] program_resource: add subroutine support (v2)
On Tue, 2015-07-21 at 15:19 +1000, Dave Airlie wrote:
> From: Dave Airlie
>
> This fleshes out the ARB_program_query support for the
> APIs that ARB_shader_subroutine introduces, leaving
> some TODOs for later addition.
>
> v2: reworked for lots of the ARB_program_interface_query
> entry points and tests
>
> Signed-off-by: Dave Airlie
> ---
> src/mesa/main/program_resource.c | 88 -
> ---
> src/mesa/main/shader_query.cpp | 82 -
>
snip
> @@ -740,6 +773,8 @@ program_resource_location(struct gl_shader_program
> *shProg,
> {
> unsigned index, offset;
> int array_index = -1;
> + long offset_ret;
> + const GLchar *base_name_end;
>
> if (res->Type == GL_PROGRAM_INPUT || res->Type == GL_PROGRAM_OUTPUT) {
>array_index = array_index_of_resource(res, name);
> @@ -780,6 +815,14 @@ program_resource_location(struct gl_shader_program
> *shProg,
>/* location in remap table + array element offset */
>return RESOURCE_UNI(res)->remap_location + offset;
>
> + case GL_VERTEX_SUBROUTINE_UNIFORM:
> + case GL_GEOMETRY_SUBROUTINE_UNIFORM:
> + case GL_FRAGMENT_SUBROUTINE_UNIFORM:
> + case GL_COMPUTE_SUBROUTINE_UNIFORM:
> + case GL_TESS_CONTROL_SUBROUTINE_UNIFORM:
> + case GL_TESS_EVALUATION_SUBROUTINE_UNIFORM:
> + offset_ret = parse_program_resource_name(name, &base_name_end);
> + return RESOURCE_UNI(res)->remap_location + ((offset_ret != -1) ?
> offset_ret : 0);
Sorry for commenting after this has already landed. But shouldn't this just
return -1 if the subscript is invalid?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix a bug where LHS swizzles of swizzles were too small.
Looks right to me. However for code clarity it would probably make sense to move: m.num_components = MAX2(m.num_components, (to + 1)); to under the second update_rhs_swizzle() call as this is the only place still using it. Reviewed-by: Timothy Arceri On Wed, 2015-07-22 at 21:39 -0700, Kenneth Graunke wrote: > A simple shader such as > >vec4 color; >color.xy.x = 1.0; > > would cause ir_assignment::set_lhs() to generate bogus IR: > >(swiz xy (swiz x (constant float (1.0 > > We were setting the number of components of each new RHS swizzle based > on the highest channel used in the LHS swizzle. So, .xy.y would > generate (swiz xy (swiz xx ...)), while .xy.x would break. > > Our existing Piglit test happened to use .xzy.z, which worked, since > 'z' is the third component, resulting in an xxx swizzle. > > This patch sets the number of swizzle components based on the size of > the LHS swizzle's inner value, so we always have the correct number > at each step. > > Fixes new Piglit tests glsl-vs-swizzle-swizzle-lhs-[23]. > Fixes ir_validate assertions in in Metro 2033 Redux. > > Cc: i...@freedesktop.org > Cc: mesa-sta...@lists.freedesktop.org > Signed-off-by: Kenneth Graunke > --- > src/glsl/ir.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp > index 8fb7ca4..7350dfa 100644 > --- a/src/glsl/ir.cpp > +++ b/src/glsl/ir.cpp > @@ -95,6 +95,8 @@ ir_assignment::set_lhs(ir_rvalue *lhs) > >write_mask |= (((this->write_mask >> i) & 1) << c); >update_rhs_swizzle(rhs_swiz, i, c); > + assert(rhs_swiz.num_components <= swiz->val->type > ->vector_elements); > + rhs_swiz.num_components = swiz->val->type->vector_elements; >} > >this->write_mask = write_mask; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V4 2/6] glsl: assign hidden uniforms their slot id earlier
On Sat, 2015-10-10 at 18:06 +0200, Marek Olšák wrote: > Hi Timothy, > > One of these 3 commits breaks compilation for Talos shaders with > gallium. My piglit patch "glsl-1.30/sampler-bug: ..." contains a > minimal test case. I can't say which commit, because Mesa fails to > build between them. > It has something to do with uniforms, structures, > and samplers. > > commit dcd9cd03837545055ce2a315e7e8840cc3254d1a > Author: Timothy Arceri > Date: Sun Aug 30 12:50:34 2015 +1000 > > glsl: store uniform slot id in var location field > Hi Marek, The piglit test passes on my intel based laptop I can't test on anything else until later this week (as I'm traveling) but my guess is its something to do with the above patch. The other two patches shouldn't change anything for gallium drivers "glsl: assign hidden uniforms their slot id earlier" just assigns hidden uniforms their slot id earlier and there shouldn't be any difference once the IR gets to glsl_to_tgsi. Also "glsl: order indices for samplers inside a struct array" shouldn't change things either as in your test the sampler are not inside the struct. There is some code in the glsl_to_tgsi pass that looks like the location field would have always been -1 for uniforms other the UBO and UBO members maybe this has something to do with the problem now that all uniforms now get a non -1 value. case ir_var_uniform: entry = new(mem_ctx) variable_storage(var, PROGRAM_UNIFORM, var->data.location); this->variables.push_tail(entry); break; I hope this helps get you started. If you haven't figured it out by later in the week than I'll take a look on my desktop once I get home. Tim > ... > commit 9788700caf61ff8beee5fd836f5efd98a931a976 > Author: Timothy Arceri > Date: Wed Sep 2 11:29:11 2015 +1000 > > glsl: assign hidden uniforms their slot id earlier > > ... > commit 874a0217fd8bba83b0bc2448f5156fdb82f77d7c > Author: Timothy Arceri > Date: Sun Aug 30 12:49:46 2015 +1000 > > glsl: order indices for samplers inside a struct array > > ... > > Any idea? > > Thanks, > Marek > > On Tue, Sep 15, 2015 at 9:51 AM, Timothy Arceri < > t_arc...@yahoo.com.au> wrote: > > This is required so that the next patch can safely assign the slot > > id > > to the var. > > > > The ids are now assigned in the order we want before allocating > > storage > > so there is no need to sort the storage array and move things > > around. > > > > V2: rename variable to make code easier to follow as suggested by > > Jason > > > > Reviewed-by: Jason Ekstrand > > --- > > src/glsl/link_uniforms.cpp | 90 +- > > > > 1 file changed, 41 insertions(+), 49 deletions(-) > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V4 2/6] glsl: assign hidden uniforms their slot id earlier
On Mon, 2015-10-12 at 01:06 +0200, Marek Olšák wrote: > On Sun, Oct 11, 2015 at 9:20 AM, Timothy Arceri < > t_arc...@yahoo.com.au> wrote: > > On Sat, 2015-10-10 at 18:06 +0200, Marek Olšák wrote: > > > Hi Timothy, > > > > > > One of these 3 commits breaks compilation for Talos shaders with > > > gallium. My piglit patch "glsl-1.30/sampler-bug: ..." contains a > > > minimal test case. I can't say which commit, because Mesa fails > > > to > > > build between them. > > > It has something to do with uniforms, structures, > > > and samplers. > > > > > > commit dcd9cd03837545055ce2a315e7e8840cc3254d1a > > > Author: Timothy Arceri > > > Date: Sun Aug 30 12:50:34 2015 +1000 > > > > > > glsl: store uniform slot id in var location field > > > > > > > Hi Marek, > > > > The piglit test passes on my intel based laptop I can't test on > > anything else until later this week (as I'm traveling) but my guess > > is > > its something to do with the above patch. > > > > The other two patches shouldn't change anything for gallium drivers > > "glsl: assign hidden uniforms their slot id earlier" just assigns > > hidden uniforms their slot id earlier and there shouldn't be any > > difference once the IR gets to glsl_to_tgsi. > > > > Also "glsl: order indices for samplers inside a struct array" > > shouldn't > > change things either as in your test the sampler are not inside the > > struct. > > > > There is some code in the glsl_to_tgsi pass that looks like the > > location field would have always been -1 for uniforms other the UBO > > and > > UBO members maybe this has something to do with the problem now > > that > > all uniforms now get a non -1 value. > > > > case ir_var_uniform: > > entry = new(mem_ctx) variable_storage(var, > > PROGRAM_UNIFORM, > >var->data.location); > > this->variables.push_tail(entry); > > break; > > > > I hope this helps get you started. If you haven't figured it out by > > later in the week than I'll take a look on my desktop once I get > > home. > > The problem is that _mesa_get_sampler_uniform_value returns a sampler > index which is greater than 16. If I allow large sampler indices, it > starts to fail with sampler.file==TGSI_FILE_NULL in the TGSI codegen > later. I didn't investigate further. Hi Marek, I just built the ilo driver to test this and I was able to get this error but only after updating master. I don't have time for a full bisect right now as I'm about to get on a flight but commit 64831832791139328a67b80387f062d39e304d24 is good and is well after this commit. Tim > > Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
