From aac90ba2920cf5ceb4df6dba776dd3952780e456 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Fri, 27 May 2016 19:07:19 +1000 Subject: [PATCH] glsl: fix xfb_offset unsized array validation This partially fixes CTS test: GL44-CTS.enhanced_layouts.xfb_get_program_resource_api The test now fails at a tes evaluation shader with unsized output arrays. The ARB_enhanced_layouts spec says: "It is a compile-time error to apply xfb_offset to the declaration of an unsized array." So this seems like a bug in the CTS. Reviewed-by: Dave Airlie --- src/compiler/glsl/ast_to_hir.cpp | 23 +++++++++++++++-------- src/compiler/glsl/ir.cpp | 23 +++++++++++++++++++++++ src/compiler/glsl/ir.h | 3 +++ src/compiler/glsl/link_varyings.cpp | 23 ----------------------- 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index 400d3c4..c0cb3d6 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -3442,11 +3442,11 @@ apply_layout_qualifier_to_variable(const struct ast_type_qualifier *qual, if (qual->flags.q.explicit_xfb_offset) { unsigned qual_xfb_offset; unsigned component_size = var->type->contains_double() ? 8 : 4; - + const glsl_type *t = get_varying_type(var, state->stage); if (process_qualifier_constant(state, loc, "xfb_offset", qual->offset, &qual_xfb_offset) && validate_xfb_offset_qualifier(loc, state, (int) qual_xfb_offset, - var->type, component_size)) { + t, component_size)) { var->data.offset = qual_xfb_offset; var->data.explicit_xfb_offset = true; } @@ -7336,12 +7336,6 @@ ast_interface_block::hir(exec_list *instructions, packing, this->block_name); - unsigned component_size = block_type->contains_double() ? 8 : 4; - int xfb_offset = - layout.flags.q.explicit_xfb_offset ? (int) qual_xfb_offset : -1; - validate_xfb_offset_qualifier(&loc, state, xfb_offset, block_type, - component_size); - if (!state->symbols->add_interface(block_type->name, block_type, var_mode)) { YYLTYPE loc = this->get_location(); _mesa_glsl_error(&loc, state, "interface block `%s' with type `%s' " @@ -7480,6 +7474,13 @@ ast_interface_block::hir(exec_list *instructions, var_mode); } + unsigned component_size = block_type->contains_double() ? 8 : 4; + int xfb_offset = + layout.flags.q.explicit_xfb_offset ? (int) qual_xfb_offset : -1; + const glsl_type *t = get_varying_type(var, state->stage); + validate_xfb_offset_qualifier(&loc, state, xfb_offset, t, + component_size); + var->data.matrix_layout = matrix_layout == GLSL_MATRIX_LAYOUT_INHERITED ? GLSL_MATRIX_LAYOUT_COLUMN_MAJOR : matrix_layout; @@ -7530,6 +7531,12 @@ ast_interface_block::hir(exec_list *instructions, */ assert(this->array_specifier == NULL); + unsigned component_size = block_type->contains_double() ? 8 : 4; + int xfb_offset = + layout.flags.q.explicit_xfb_offset ? (int) qual_xfb_offset : -1; + validate_xfb_offset_qualifier(&loc, state, xfb_offset, block_type, + component_size); + for (unsigned i = 0; i < num_variables; i++) { ir_variable *var = new(state) ir_variable(fields[i].type, diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp index 5bb3ac3..70859a7 100644 --- a/src/compiler/glsl/ir.cpp +++ b/src/compiler/glsl/ir.cpp @@ -2021,3 +2021,26 @@ mode_string(const ir_variable *var) assert(!"Should not get here."); return "invalid variable"; } + +/** + * Get the varying type stripped of the outermost array if we're processing + * a stage whose varyings are arrays indexed by a vertex number (such as + * geometry shader inputs). + */ +const glsl_type * +get_varying_type(const ir_variable *var, gl_shader_stage stage) +{ + const glsl_type *type = var->type; + + if (!var->data.patch && + ((var->data.mode == ir_var_shader_out && + stage == MESA_SHADER_TESS_CTRL) || + (var->data.mode == ir_var_shader_in && + (stage == MESA_SHADER_TESS_CTRL || stage == MESA_SHADER_TESS_EVAL || + stage == MESA_SHADER_GEOMETRY)))) { + assert(type->is_array()); + type = type->fields.array; + } + + return type; +} diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h index e8efd27..b1cfd52 100644 --- a/src/compiler/glsl/ir.h +++ b/src/compiler/glsl/ir.h @@ -2621,6 +2621,9 @@ is_gl_identifier(const char *s) return s && s[0] == 'g' && s[1] == 'l' && s[2] == '_'; } +const glsl_type * +get_varying_type(const ir_variable *var, gl_shader_stage stage); + extern "C" { #endif /* __cplusplus */ diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index 7c3bedf..34c8906 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -40,29 +40,6 @@ #include "program.h" -/** - * Get the varying type stripped of the outermost array if we're processing - * a stage whose varyings are arrays indexed by a vertex number (such as - * geometry shader inputs). - */ -static const glsl_type * -get_varying_type(const ir_variable *var, gl_shader_stage stage) -{ - const glsl_type *type = var->type; - - if (!var->data.patch && - ((var->data.mode == ir_var_shader_out && - stage == MESA_SHADER_TESS_CTRL) || - (var->data.mode == ir_var_shader_in && - (stage == MESA_SHADER_TESS_CTRL || stage == MESA_SHADER_TESS_EVAL || - stage == MESA_SHADER_GEOMETRY)))) { - assert(type->is_array()); - type = type->fields.array; - } - - return type; -} - static void create_xfb_varying_names(void *mem_ctx, const glsl_type *t, char **name, size_t name_length, unsigned *count, -- 2.7.4