From f1b647fdd1028bf475ed258c4dd8b833339ec796 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Tue, 28 Apr 2015 12:09:58 +0200 Subject: [PATCH] glsl: Apply memory qualifiers to buffer variables MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit v2: - Save memory qualifier info in the top level members of a shader storage block. - Add a checks to record_compare() which is used when comparing shader storage buffer declarations in different shaders. - Always report an error for incompatible readonly/writeonly definitions, whether they are present at block or field level. Reviewed-by: Kristian Høgsberg --- src/glsl/ast_to_hir.cpp | 63 +++++++++++++++++++++++++++++++++++++++-- src/glsl/glsl_types.cpp | 20 +++++++++++++ src/glsl/glsl_types.h | 11 +++++++ 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 97c6350b1f0..0b8b4016a7e 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -5606,10 +5606,19 @@ ast_process_structure_or_interface_block(exec_list *instructions, bool is_interface, enum glsl_matrix_layout matrix_layout, bool allow_reserved_names, - ir_variable_mode var_mode) + ir_variable_mode var_mode, + ast_type_qualifier *layout) { unsigned decl_count = 0; + /* For blocks that accept memory qualifiers (i.e. shader storage), verify + * that we don't have incompatible qualifiers + */ + if (layout && layout->flags.q.read_only && layout->flags.q.write_only) { + _mesa_glsl_error(&loc, state, + "Interface block sets both readonly and writeonly"); + } + /* Make an initial pass over the list of fields to determine how * many there are. Each element in this list is an ast_declarator_list. * This means that we actually need to count the number of elements in the @@ -5771,6 +5780,44 @@ ast_process_structure_or_interface_block(exec_list *instructions, || fields[i].matrix_layout == GLSL_MATRIX_LAYOUT_COLUMN_MAJOR); } + /* Image qualifiers are allowed on buffer variables, which can only + * 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. + */ + bool read_only = layout->flags.q.read_only; + bool write_only = layout->flags.q.write_only; + + if (qual->flags.q.read_only) { + read_only = true; + write_only = false; + } else if (qual->flags.q.write_only) { + read_only = false; + write_only = true; + } + + fields[i].image_read_only = read_only; + fields[i].image_write_only = write_only; + + /* For other qualifiers, we set the flag if either the layout + * qualifier or the field qualifier are set + */ + fields[i].image_coherent = qual->flags.q.coherent || + layout->flags.q.coherent; + fields[i].image_volatile = qual->flags.q._volatile || + layout->flags.q._volatile; + fields[i].image_restrict = qual->flags.q.restrict_flag || + layout->flags.q.restrict_flag; + } + i++; } } @@ -5825,7 +5872,8 @@ ast_struct_specifier::hir(exec_list *instructions, false, GLSL_MATRIX_LAYOUT_INHERITED, false /* allow_reserved_names */, - ir_var_auto); + ir_var_auto, + NULL); validate_identifier(this->name, loc, state); @@ -5980,7 +6028,8 @@ ast_interface_block::hir(exec_list *instructions, true, matrix_layout, redeclaring_per_vertex, - var_mode); + var_mode, + &this->layout); state->struct_specifier_depth--; @@ -6364,6 +6413,14 @@ ast_interface_block::hir(exec_list *instructions, var->data.stream = this->layout.stream; + if (var->data.mode == ir_var_shader_storage) { + var->data.image_read_only = fields[i].image_read_only; + var->data.image_write_only = fields[i].image_write_only; + var->data.image_coherent = fields[i].image_coherent; + var->data.image_volatile = fields[i].image_volatile; + var->data.image_restrict = fields[i].image_restrict; + } + /* Examine var name here since var may get deleted in the next call */ bool var_is_gl_id = is_gl_identifier(var->name); diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 93034a67f01..0ead0f2a327 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -124,6 +124,11 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields, this->fields.structure[i].sample = fields[i].sample; this->fields.structure[i].matrix_layout = fields[i].matrix_layout; this->fields.structure[i].patch = fields[i].patch; + this->fields.structure[i].image_read_only = fields[i].image_read_only; + this->fields.structure[i].image_write_only = fields[i].image_write_only; + this->fields.structure[i].image_coherent = fields[i].image_coherent; + this->fields.structure[i].image_volatile = fields[i].image_volatile; + this->fields.structure[i].image_restrict = fields[i].image_restrict; } mtx_unlock(&glsl_type::mutex); @@ -760,6 +765,21 @@ glsl_type::record_compare(const glsl_type *b) const if (this->fields.structure[i].patch != b->fields.structure[i].patch) return false; + if (this->fields.structure[i].image_read_only + != b->fields.structure[i].image_read_only) + return false; + if (this->fields.structure[i].image_write_only + != b->fields.structure[i].image_write_only) + return false; + if (this->fields.structure[i].image_coherent + != b->fields.structure[i].image_coherent) + return false; + if (this->fields.structure[i].image_volatile + != b->fields.structure[i].image_volatile) + return false; + if (this->fields.structure[i].image_restrict + != b->fields.structure[i].image_restrict) + return false; } return true; diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h index d58d8189e21..23ada15b854 100644 --- a/src/glsl/glsl_types.h +++ b/src/glsl/glsl_types.h @@ -810,6 +810,17 @@ struct glsl_struct_field { */ int stream; + + /** + * Image qualifiers, applicable to buffer variables defined in shader + * storage buffer objects (SSBOs) + */ + unsigned image_read_only:1; + unsigned image_write_only:1; + unsigned image_coherent:1; + unsigned image_volatile:1; + unsigned image_restrict:1; + glsl_struct_field(const struct glsl_type *_type, const char *_name) : type(_type), name(_name), location(-1), interpolation(0), centroid(0), sample(0), matrix_layout(GLSL_MATRIX_LAYOUT_INHERITED), patch(0), -- 2.30.2