[Mesa-dev] [PATCH v2 1/4] glsl: look for multiple variables simultaneously with find_assignment_visitor
Nicolai Hähnle
nhaehnle at gmail.com
Tue Jul 4 09:37:38 UTC 2017
On 03.07.2017 14:34, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> Save some passes over the IR.
>
> v2: redesign to make the users of find_assignments more readable
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com> (v1)
> ---
> src/compiler/glsl/linker.cpp | 120 ++++++++++++++++++++++++++++---------------
> 1 file changed, 79 insertions(+), 41 deletions(-)
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 15295da..4aa4682 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -83,82 +83,120 @@
> #include "ir_uniform.h"
> #include "builtin_functions.h"
> #include "shader_cache.h"
>
> #include "main/shaderobj.h"
> #include "main/enums.h"
>
>
> namespace {
>
> +struct find_variable {
> + const char *name;
> + bool found;
> +
> + find_variable(const char *name) : name(name), found(false) {}
> +};
> +
> /**
> * Visitor that determines whether or not a variable is ever written.
> + *
> + * Use \ref find_assignments for convenience.
> */
> class find_assignment_visitor : public ir_hierarchical_visitor {
> public:
> - find_assignment_visitor(const char *name)
> - : name(name), found(false)
> + find_assignment_visitor(unsigned num_vars,
> + find_variable * const *vars)
> + : num_variables(num_vars), num_found(0), variables(vars)
> {
> - /* empty */
> }
>
> virtual ir_visitor_status visit_enter(ir_assignment *ir)
> {
> ir_variable *const var = ir->lhs->variable_referenced();
>
> - if (strcmp(name, var->name) == 0) {
> - found = true;
> - return visit_stop;
> - }
> -
> - return visit_continue_with_parent;
> + return check_variable_name(var->name);
> }
>
> virtual ir_visitor_status visit_enter(ir_call *ir)
> {
> foreach_two_lists(formal_node, &ir->callee->parameters,
> actual_node, &ir->actual_parameters) {
> ir_rvalue *param_rval = (ir_rvalue *) actual_node;
> ir_variable *sig_param = (ir_variable *) formal_node;
>
> if (sig_param->data.mode == ir_var_function_out ||
> sig_param->data.mode == ir_var_function_inout) {
> ir_variable *var = param_rval->variable_referenced();
> - if (var && strcmp(name, var->name) == 0) {
> - found = true;
> + if (var && check_variable_name(var->name) == visit_stop)
> return visit_stop;
> - }
> }
> }
>
> if (ir->return_deref != NULL) {
> ir_variable *const var = ir->return_deref->variable_referenced();
>
> - if (strcmp(name, var->name) == 0) {
> - found = true;
> + if (check_variable_name(var->name) == visit_stop)
> return visit_stop;
> - }
> }
>
> return visit_continue_with_parent;
> }
>
> - bool variable_found()
> +private:
> + ir_visitor_status check_variable_name(const char *name)
> {
> - return found;
> + for (unsigned i = 0; i < num_variables; ++i) {
> + if (strcmp(variables[i]->name, name) == 0) {
> + if (variables[i]->found) {
This should be !variables[i]->found. Fixed locally.
Cheers,
Nicolai
> + variables[i]->found = true;
> + if (++num_found >= num_variables)
> + return visit_stop;
> + }
> + break;
> + }
> + }
> +
> + return visit_continue_with_parent;
> }
>
> private:
> - const char *name; /**< Find writes to a variable with this name. */
> - bool found; /**< Was a write to the variable found? */
> + unsigned num_variables;
> + unsigned num_found;
> + find_variable * const *variables;
> };
>
> +/**
> + * Determine whether or not any of NULL-terminated list of variables is ever
> + * written to.
> + */
> +static void
> +find_assignments(exec_list *ir, find_variable * const *vars)
> +{
> + unsigned num_variables = 0;
> +
> + for (find_variable * const *v = vars; *v; ++v)
> + num_variables++;
> +
> + find_assignment_visitor visitor(num_variables, vars);
> + visitor.run(ir);
> +}
> +
> +/**
> + * Determine whether or not the given variable is ever written to.
> + */
> +static void
> +find_assignments(exec_list *ir, find_variable *var)
> +{
> + find_assignment_visitor visitor(1, &var);
> + visitor.run(ir);
> +}
>
> /**
> * Visitor that determines whether or not a variable is ever read.
> */
> class find_deref_visitor : public ir_hierarchical_visitor {
> public:
> find_deref_visitor(const char *name)
> : name(name), found(false)
> {
> /* empty */
> @@ -560,61 +598,62 @@ analyze_clip_cull_usage(struct gl_shader_program *prog,
> /* From section 7.1 (Vertex Shader Special Variables) of the
> * GLSL 1.30 spec:
> *
> * "It is an error for a shader to statically write both
> * gl_ClipVertex and gl_ClipDistance."
> *
> * This does not apply to GLSL ES shaders, since GLSL ES defines neither
> * gl_ClipVertex nor gl_ClipDistance. However with
> * GL_EXT_clip_cull_distance, this functionality is exposed in ES 3.0.
> */
> - find_assignment_visitor clip_distance("gl_ClipDistance");
> - find_assignment_visitor cull_distance("gl_CullDistance");
> -
> - clip_distance.run(shader->ir);
> - cull_distance.run(shader->ir);
> + find_variable gl_ClipDistance("gl_ClipDistance");
> + find_variable gl_CullDistance("gl_CullDistance");
> + find_variable gl_ClipVertex("gl_ClipVertex");
> + find_variable * const variables[] = {
> + &gl_ClipDistance,
> + &gl_CullDistance,
> + !prog->IsES ? &gl_ClipVertex : NULL,
> + NULL
> + };
> + find_assignments(shader->ir, variables);
>
> /* From the ARB_cull_distance spec:
> *
> * It is a compile-time or link-time error for the set of shaders forming
> * a program to statically read or write both gl_ClipVertex and either
> * gl_ClipDistance or gl_CullDistance.
> *
> * This does not apply to GLSL ES shaders, since GLSL ES doesn't define
> * gl_ClipVertex.
> */
> if (!prog->IsES) {
> - find_assignment_visitor clip_vertex("gl_ClipVertex");
> -
> - clip_vertex.run(shader->ir);
> -
> - if (clip_vertex.variable_found() && clip_distance.variable_found()) {
> + if (gl_ClipVertex.found && gl_ClipDistance.found) {
> linker_error(prog, "%s shader writes to both `gl_ClipVertex' "
> "and `gl_ClipDistance'\n",
> _mesa_shader_stage_to_string(shader->Stage));
> return;
> }
> - if (clip_vertex.variable_found() && cull_distance.variable_found()) {
> + if (gl_ClipVertex.found && gl_CullDistance.found) {
> linker_error(prog, "%s shader writes to both `gl_ClipVertex' "
> "and `gl_CullDistance'\n",
> _mesa_shader_stage_to_string(shader->Stage));
> return;
> }
> }
>
> - if (clip_distance.variable_found()) {
> + if (gl_ClipDistance.found) {
> ir_variable *clip_distance_var =
> shader->symbols->get_variable("gl_ClipDistance");
> assert(clip_distance_var);
> *clip_distance_array_size = clip_distance_var->type->length;
> }
> - if (cull_distance.variable_found()) {
> + if (gl_CullDistance.found) {
> ir_variable *cull_distance_var =
> shader->symbols->get_variable("gl_CullDistance");
> assert(cull_distance_var);
> *cull_distance_array_size = cull_distance_var->type->length;
> }
> /* From the ARB_cull_distance spec:
> *
> * It is a compile-time or link-time error for the set of shaders forming
> * a program to have the sum of the sizes of the gl_ClipDistance and
> * gl_CullDistance arrays to be larger than
> @@ -669,23 +708,23 @@ validate_vertex_shader_executable(struct gl_shader_program *prog,
> * after being written. This value will be used by primitive
> * assembly, clipping, culling, and other fixed functionality
> * operations, if present, that operate on primitives after
> * vertex processing has occurred. Its value is undefined if
> * the vertex shader executable does not write gl_Position."
> *
> * All GLSL ES Versions are similar to GLSL 1.40--failing to write to
> * gl_Position is not an error.
> */
> if (prog->data->Version < (prog->IsES ? 300 : 140)) {
> - find_assignment_visitor find("gl_Position");
> - find.run(shader->ir);
> - if (!find.variable_found()) {
> + find_variable gl_Position("gl_Position");
> + find_assignments(shader->ir, &gl_Position);
> + if (!gl_Position.found) {
> if (prog->IsES) {
> linker_warning(prog,
> "vertex shader does not write to `gl_Position'. "
> "Its value is undefined. \n");
> } else {
> linker_error(prog,
> "vertex shader does not write to `gl_Position'. \n");
> }
> return;
> }
> @@ -715,27 +754,26 @@ validate_tess_eval_shader_executable(struct gl_shader_program *prog,
> *
> * \param shader Fragment shader executable to be verified
> */
> void
> validate_fragment_shader_executable(struct gl_shader_program *prog,
> struct gl_linked_shader *shader)
> {
> if (shader == NULL)
> return;
>
> - find_assignment_visitor frag_color("gl_FragColor");
> - find_assignment_visitor frag_data("gl_FragData");
> -
> - frag_color.run(shader->ir);
> - frag_data.run(shader->ir);
> + find_variable gl_FragColor("gl_FragColor");
> + find_variable gl_FragData("gl_FragData");
> + find_variable * const variables[] = { &gl_FragColor, &gl_FragData, NULL };
> + find_assignments(shader->ir, variables);
>
> - if (frag_color.variable_found() && frag_data.variable_found()) {
> + if (gl_FragColor.found && gl_FragData.found) {
> linker_error(prog, "fragment shader writes to both "
> "`gl_FragColor' and `gl_FragData'\n");
> }
> }
>
> /**
> * Verify that a geometry shader executable meets all semantic requirements
> *
> * Also sets prog->Geom.VerticesIn, and info.clip_distance_array_sizeand
> * info.cull_distance_array_size as a side effect.
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list