Mesa (master): glsl: only do type and qualifier validation once per declaration
Timothy Arceri
tarceri at kemper.freedesktop.org
Tue Nov 17 20:31:03 UTC 2015
Module: Mesa
Branch: master
Commit: c54865db784ec26406aa98ebe67d86568ab9fc96
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=c54865db784ec26406aa98ebe67d86568ab9fc96
Author: Timothy Arceri <timothy.arceri at collabora.com>
Date: Fri Nov 13 10:27:00 2015 +1100
glsl: only do type and qualifier validation once per declaration
For struct and block members previously we were doing it for
every variable declaration.
So for example
struct S {
atomic_uint x, y, z;
};
Would previously generate three error messages when one is sufficient.
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
---
src/glsl/ast_to_hir.cpp | 196 +++++++++++++++++++++++------------------------
1 file changed, 97 insertions(+), 99 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index ea7b2c4..b553a0d 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -6102,74 +6102,114 @@ ast_process_struct_or_iface_block_members(exec_list *instructions,
const glsl_type *decl_type =
decl_list->type->glsl_type(& type_name, state);
- foreach_list_typed (ast_declaration, decl, link,
- &decl_list->declarations) {
- if (!allow_reserved_names)
- validate_identifier(decl->identifier, loc, state);
+ const struct ast_type_qualifier *const qual =
+ &decl_list->type->qualifier;
- /* From section 4.3.9 of the GLSL 4.40 spec:
- *
- * "[In interface blocks] opaque types are not allowed."
+ /* From section 4.3.9 of the GLSL 4.40 spec:
+ *
+ * "[In interface blocks] opaque types are not allowed."
+ *
+ * It should be impossible for decl_type to be NULL here. Cases that
+ * might naturally lead to decl_type being NULL, especially for the
+ * is_interface case, will have resulted in compilation having
+ * already halted due to a syntax error.
+ */
+ assert(decl_type);
+
+ if (is_interface && decl_type->contains_opaque()) {
+ YYLTYPE loc = decl_list->get_location();
+ _mesa_glsl_error(&loc, state,
+ "uniform/buffer in non-default interface block contains "
+ "opaque variable");
+ }
+
+ if (decl_type->contains_atomic()) {
+ /* From section 4.1.7.3 of the GLSL 4.40 spec:
*
- * It should be impossible for decl_type to be NULL here. Cases that
- * might naturally lead to decl_type being NULL, especially for the
- * is_interface case, will have resulted in compilation having
- * already halted due to a syntax error.
+ * "Members of structures cannot be declared as atomic counter
+ * types."
*/
- assert(decl_type);
+ YYLTYPE loc = decl_list->get_location();
+ _mesa_glsl_error(&loc, state, "atomic counter in structure, "
+ "shader storage block or uniform block");
+ }
- if (is_interface && decl_type->contains_opaque()) {
- YYLTYPE loc = decl_list->get_location();
- _mesa_glsl_error(&loc, state,
- "uniform/buffer in non-default interface block contains "
- "opaque variable");
- }
+ if (decl_type->contains_image()) {
+ /* FINISHME: Same problem as with atomic counters.
+ * FINISHME: Request clarification from Khronos and add
+ * FINISHME: spec quotation here.
+ */
+ YYLTYPE loc = decl_list->get_location();
+ _mesa_glsl_error(&loc, state,
+ "image in structure, shader storage block or "
+ "uniform block");
+ }
- if (decl_type->contains_atomic()) {
- /* From section 4.1.7.3 of the GLSL 4.40 spec:
- *
- * "Members of structures cannot be declared as atomic counter
- * types."
- */
- YYLTYPE loc = decl_list->get_location();
- _mesa_glsl_error(&loc, state, "atomic counter in structure, "
- "shader storage block or uniform block");
- }
+ if (qual->flags.q.explicit_binding)
+ validate_binding_qualifier(state, &loc, decl_type, qual);
- if (decl_type->contains_image()) {
- /* FINISHME: Same problem as with atomic counters.
- * FINISHME: Request clarification from Khronos and add
- * FINISHME: spec quotation here.
- */
- YYLTYPE loc = decl_list->get_location();
- _mesa_glsl_error(&loc, state,
- "image in structure, shader storage block or "
- "uniform block");
- }
+ if (qual->flags.q.std140 ||
+ qual->flags.q.std430 ||
+ qual->flags.q.packed ||
+ qual->flags.q.shared) {
+ _mesa_glsl_error(&loc, state,
+ "uniform/shader storage block layout qualifiers "
+ "std140, std430, packed, and shared can only be "
+ "applied to uniform/shader storage blocks, not "
+ "members");
+ }
- const struct ast_type_qualifier *const qual =
- & decl_list->type->qualifier;
+ if (qual->flags.q.constant) {
+ YYLTYPE loc = decl_list->get_location();
+ _mesa_glsl_error(&loc, state,
+ "const storage qualifier cannot be applied "
+ "to struct or interface block members");
+ }
- if (qual->flags.q.explicit_binding)
- validate_binding_qualifier(state, &loc, decl_type, qual);
+ /* From Section 4.4.2.3 (Geometry Outputs) of the GLSL 4.50 spec:
+ *
+ * "A block member may be declared with a stream identifier, but
+ * the specified stream must match the stream associated with the
+ * containing block."
+ */
+ if (qual->flags.q.explicit_stream &&
+ qual->stream != layout->stream) {
+ _mesa_glsl_error(&loc, state, "stream layout qualifier on interface "
+ "block member does not match the interface block "
+ "(%d vs %d)", qual->stream, layout->stream);
+ }
- if (qual->flags.q.std140 ||
- qual->flags.q.std430 ||
- qual->flags.q.packed ||
- qual->flags.q.shared) {
- _mesa_glsl_error(&loc, state,
- "uniform/shader storage block layout qualifiers "
- "std140, std430, packed, and shared can only be "
- "applied to uniform/shader storage blocks, not "
- "members");
- }
+ if (qual->flags.q.uniform && qual->has_interpolation()) {
+ _mesa_glsl_error(&loc, state,
+ "interpolation qualifiers cannot be used "
+ "with uniform interface blocks");
+ }
+
+ if ((qual->flags.q.uniform || !is_interface) &&
+ qual->has_auxiliary_storage()) {
+ _mesa_glsl_error(&loc, state,
+ "auxiliary storage qualifiers cannot be used "
+ "in uniform blocks or structures.");
+ }
- if (qual->flags.q.constant) {
- YYLTYPE loc = decl_list->get_location();
+ if (qual->flags.q.row_major || qual->flags.q.column_major) {
+ if (!qual->flags.q.uniform && !qual->flags.q.buffer) {
_mesa_glsl_error(&loc, state,
- "const storage qualifier cannot be applied "
- "to struct or interface block members");
- }
+ "row_major and column_major can only be "
+ "applied to interface blocks");
+ } else
+ validate_matrix_layout_for_type(state, &loc, decl_type, NULL);
+ }
+
+ if (qual->flags.q.read_only && qual->flags.q.write_only) {
+ _mesa_glsl_error(&loc, state, "buffer variable can't be both "
+ "readonly and writeonly.");
+ }
+
+ foreach_list_typed (ast_declaration, decl, link,
+ &decl_list->declarations) {
+ if (!allow_reserved_names)
+ validate_identifier(decl->identifier, loc, state);
const struct glsl_type *field_type =
process_array_type(&loc, decl_type, decl->array_specifier, state);
@@ -6184,42 +6224,6 @@ ast_process_struct_or_iface_block_members(exec_list *instructions,
fields[i].patch = qual->flags.q.patch ? 1 : 0;
fields[i].precision = qual->precision;
- /* From Section 4.4.2.3 (Geometry Outputs) of the GLSL 4.50 spec:
- *
- * "A block member may be declared with a stream identifier, but
- * the specified stream must match the stream associated with the
- * containing block."
- */
- if (qual->flags.q.explicit_stream &&
- qual->stream != layout->stream) {
- _mesa_glsl_error(&loc, state, "stream layout qualifier on "
- "interface block member `%s' does not match "
- "the interface block (%d vs %d)",
- fields[i].name, qual->stream, layout->stream);
- }
-
- if (qual->flags.q.row_major || qual->flags.q.column_major) {
- if (!qual->flags.q.uniform && !qual->flags.q.buffer) {
- _mesa_glsl_error(&loc, state,
- "row_major and column_major can only be "
- "applied to interface blocks");
- } else
- validate_matrix_layout_for_type(state, &loc, field_type, NULL);
- }
-
- if (qual->flags.q.uniform && qual->has_interpolation()) {
- _mesa_glsl_error(&loc, state,
- "interpolation qualifiers cannot be used "
- "with uniform interface blocks");
- }
-
- if ((qual->flags.q.uniform || !is_interface) &&
- qual->has_auxiliary_storage()) {
- _mesa_glsl_error(&loc, state,
- "auxiliary storage qualifiers cannot be used "
- "in uniform blocks or structures.");
- }
-
/* Propogate row- / column-major information down the fields of the
* structure or interface block. Structures need this data because
* the structure may contain a structure that contains ... a matrix
@@ -6249,12 +6253,6 @@ ast_process_struct_or_iface_block_members(exec_list *instructions,
* be defined inside shader storage buffer objects
*/
if (layout && var_mode == ir_var_shader_storage) {
- if (qual->flags.q.read_only && qual->flags.q.write_only) {
- _mesa_glsl_error(&loc, state,
- "buffer variable `%s' can't be "
- "readonly and writeonly.", fields[i].name);
- }
-
/* For readonly and writeonly qualifiers the field definition,
* if set, overwrites the layout qualifier.
*/
More information about the mesa-commit
mailing list