Module: Mesa Branch: main Commit: 792c9a0a24c8d46c0f1ee58162c0c24cc8fb228b URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=792c9a0a24c8d46c0f1ee58162c0c24cc8fb228b
Author: Timothy Arceri <[email protected]> Date: Wed May 11 22:53:46 2022 +1000 glsl: move validation of sampler indirects to the nir linker This will allow us to disable the GLSL IR loop unroller in a following patch and rely on the NIR loop unroller instead. This allows the piglit test spec@!opengl 2.0@max-samplers border to pass on the v3d rpi4 driver. Reviewed-by: Emma Anholt <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16543> --- src/broadcom/ci/broadcom-rpi4-fails.txt | 2 - src/compiler/glsl/gl_nir_linker.c | 87 +++++++++++++++++++++++++++++++++ src/compiler/glsl/linker.cpp | 80 +----------------------------- 3 files changed, 88 insertions(+), 81 deletions(-) diff --git a/src/broadcom/ci/broadcom-rpi4-fails.txt b/src/broadcom/ci/broadcom-rpi4-fails.txt index 695c9162cad..8d324eeced0 100644 --- a/src/broadcom/ci/broadcom-rpi4-fails.txt +++ b/src/broadcom/ci/broadcom-rpi4-fails.txt @@ -96,8 +96,6 @@ spec@!opengl 1.1@texwrap formats bordercolor-swizzled@GL_RGBA16- swizzled- borde spec@!opengl [email protected],Fail spec@!opengl [email protected],Fail spec@!opengl [email protected],Fail -spec@!opengl 2.0@max-samplers,Fail -spec@!opengl 2.0@max-samplers border,Fail spec@!opengl 2.1@pbo,Fail spec@!opengl 2.1@pbo@test_polygon_stip,Fail spec@!opengl 2.1@polygon-stipple-fs,Fail diff --git a/src/compiler/glsl/gl_nir_linker.c b/src/compiler/glsl/gl_nir_linker.c index de1a7927680..da2708343ed 100644 --- a/src/compiler/glsl/gl_nir_linker.c +++ b/src/compiler/glsl/gl_nir_linker.c @@ -778,6 +778,83 @@ check_image_resources(const struct gl_constants *consts, " buffers and fragment outputs\n"); } +static bool +is_sampler_array_accessed_indirectly(nir_deref_instr *deref) +{ + for (nir_deref_instr *d = deref; d; d = nir_deref_instr_parent(d)) { + if (d->deref_type != nir_deref_type_array) + continue; + + if (nir_src_is_const(d->arr.index)) + continue; + + return true; + } + + return false; +} + +/** + * This check is done to make sure we allow only constant expression + * indexing and "constant-index-expression" (indexing with an expression + * that includes loop induction variable). + */ +static bool +validate_sampler_array_indexing(const struct gl_constants *consts, + struct gl_shader_program *prog) +{ + for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { + if (prog->_LinkedShaders[i] == NULL) + continue; + + bool no_dynamic_indexing = + consts->ShaderCompilerOptions[i].NirOptions->force_indirect_unrolling_sampler; + + bool uses_indirect_sampler_array_indexing = false; + nir_foreach_function(function, prog->_LinkedShaders[i]->Program->nir) { + nir_foreach_block(block, function->impl) { + nir_foreach_instr(instr, block) { + /* Check if a sampler array is accessed indirectly */ + if (instr->type == nir_instr_type_tex) { + nir_tex_instr *tex_instr = nir_instr_as_tex(instr); + int sampler_idx = + nir_tex_instr_src_index(tex_instr, nir_tex_src_sampler_deref); + if (sampler_idx >= 0) { + nir_deref_instr *deref = + nir_instr_as_deref(tex_instr->src[sampler_idx].src.ssa->parent_instr); + if (is_sampler_array_accessed_indirectly(deref)) { + uses_indirect_sampler_array_indexing = true; + break; + } + } + } + } + + if (uses_indirect_sampler_array_indexing) + break; + } + if (uses_indirect_sampler_array_indexing) + break; + } + + if (uses_indirect_sampler_array_indexing) { + const char *msg = "sampler arrays indexed with non-constant " + "expressions is forbidden in GLSL %s %u"; + /* Backend has indicated that it has no dynamic indexing support. */ + if (no_dynamic_indexing) { + linker_error(prog, msg, prog->IsES ? "ES" : "", + prog->data->Version); + return false; + } else { + linker_warning(prog, msg, prog->IsES ? "ES" : "", + prog->data->Version); + } + } + } + + return true; +} + bool gl_nir_link_glsl(const struct gl_constants *consts, const struct gl_extensions *exts, @@ -790,6 +867,16 @@ gl_nir_link_glsl(const struct gl_constants *consts, if (!gl_nir_link_varyings(consts, exts, api, prog)) return false; + /* Validation for special cases where we allow sampler array indexing + * with loop induction variable. This check emits a warning or error + * depending if backend can handle dynamic indexing. + */ + if ((!prog->IsES && prog->data->Version < 130) || + (prog->IsES && prog->data->Version < 300)) { + if (!validate_sampler_array_indexing(consts, prog)) + return false; + } + for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { struct gl_linked_shader *shader = prog->_LinkedShaders[i]; if (shader) { diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index ade1f4ff5b0..5a94e182cc1 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -69,6 +69,7 @@ #include "glsl_symbol_table.h" #include "glsl_parser_extras.h" #include "ir.h" +#include "nir.h" #include "program.h" #include "program/prog_instruction.h" #include "program/program.h" @@ -450,39 +451,6 @@ private: unsigned used_streams; }; -/* Class that finds array derefs and check if indexes are dynamic. */ -class dynamic_sampler_array_indexing_visitor : public ir_hierarchical_visitor -{ -public: - dynamic_sampler_array_indexing_visitor() : - dynamic_sampler_array_indexing(false) - { - } - - ir_visitor_status visit_enter(ir_dereference_array *ir) - { - if (!ir->variable_referenced()) - return visit_continue; - - if (!ir->variable_referenced()->type->contains_sampler()) - return visit_continue; - - if (!ir->array_index->constant_expression_value(ralloc_parent(ir))) { - dynamic_sampler_array_indexing = true; - return visit_stop; - } - return visit_continue; - } - - bool uses_dynamic_sampler_array_indexing() - { - return dynamic_sampler_array_indexing; - } - -private: - bool dynamic_sampler_array_indexing; -}; - } /* anonymous namespace */ void @@ -3383,42 +3351,6 @@ check_explicit_uniform_locations(const struct gl_extensions *exts, prog->NumExplicitUniformLocations = entries_total; } -/** - * This check is done to make sure we allow only constant expression - * indexing and "constant-index-expression" (indexing with an expression - * that includes loop induction variable). - */ -static bool -validate_sampler_array_indexing(const struct gl_constants *consts, - struct gl_shader_program *prog) -{ - dynamic_sampler_array_indexing_visitor v; - for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { - if (prog->_LinkedShaders[i] == NULL) - continue; - - bool no_dynamic_indexing = - consts->ShaderCompilerOptions[i].EmitNoIndirectSampler; - - /* Search for array derefs in shader. */ - v.run(prog->_LinkedShaders[i]->ir); - if (v.uses_dynamic_sampler_array_indexing()) { - const char *msg = "sampler arrays indexed with non-constant " - "expressions is forbidden in GLSL %s %u"; - /* Backend has indicated that it has no dynamic indexing support. */ - if (no_dynamic_indexing) { - linker_error(prog, msg, prog->IsES ? "ES" : "", - prog->data->Version); - return false; - } else { - linker_warning(prog, msg, prog->IsES ? "ES" : "", - prog->data->Version); - } - } - } - return true; -} - static void link_assign_subroutine_types(struct gl_shader_program *prog) { @@ -3992,16 +3924,6 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) } - /* Validation for special cases where we allow sampler array indexing - * with loop induction variable. This check emits a warning or error - * depending if backend can handle dynamic indexing. - */ - if ((!prog->IsES && prog->data->Version < 130) || - (prog->IsES && prog->data->Version < 300)) { - if (!validate_sampler_array_indexing(consts, prog)) - goto done; - } - /* Check and validate stream emissions in geometry shaders */ validate_geometry_shader_emissions(consts, prog);
