[Mesa-dev] [PATCH 3/9] glsl: look for multiple variables simultaneously with find_assignment_visitor
Timothy Arceri
tarceri at itsqueeze.com
Mon Jun 26 10:18:00 UTC 2017
On 26/06/17 19:40, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> Save some passes over the IR.
> ---
> src/compiler/glsl/linker.cpp | 88 +++++++++++++++++++++++++-------------------
> 1 file changed, 50 insertions(+), 38 deletions(-)
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 5366200..cfda263 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -88,75 +88,82 @@
> #include "main/enums.h"
>
>
> namespace {
>
> /**
> * Visitor that determines whether or not a variable is ever written.
> */
> class find_assignment_visitor : public ir_hierarchical_visitor {
> public:
> - find_assignment_visitor(const char *name)
> - : name(name), found(false)
> + find_assignment_visitor(unsigned int num_names, const char * const *names)
> + : num_names(num_names), names(names), found(0)
> {
> - /* empty */
> + assert(num_names <= 32);
> }
>
> 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()
> + bool variable_found(unsigned idx)
> {
> - return found;
> + return found & (1u << idx);
> }
>
> private:
> - const char *name; /**< Find writes to a variable with this name. */
> - bool found; /**< Was a write to the variable found? */
> + ir_visitor_status check_variable_name(const char *name)
> + {
> + for (unsigned i = 0; i < num_names; ++i) {
> + if (strcmp(names[i], name) == 0) {
> + found |= 1u << i;
> + if (found == u_bit_consecutive(0, num_names))
> + return visit_stop;
> + break;
> + }
> + }
> +
> + return visit_continue_with_parent;
> + }
> +
> +private:
> + unsigned int num_names;
> + const char * const * names; /**< Find writes to variables with these names. */
> + uint32_t found; /**< Was a write to the variable found? */
> };
>
>
> /**
> * 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)
> @@ -560,61 +567,63 @@ 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");
> + static const char * const variable_names[] = {
> + "gl_ClipDistance",
> + "gl_CullDistance",
> + "gl_ClipVertex"
> + };
> +
> + find_assignment_visitor clipcull_visitor(!prog->IsES ? 3 : 2, variable_names);
>
> - clip_distance.run(shader->ir);
> - cull_distance.run(shader->ir);
> + clipcull_visitor.run(shader->ir);
>
> /* 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 (clipcull_visitor.variable_found(2) &&
> + clipcull_visitor.variable_found(0)) {
The 0, 2 etc is kind of confusing if you are browsing the code without
knowledge of where the come from. Although I'm not sure what to suggest.
Maybe Emil or someone else can comment of patch 6. Everything else (same
comment in patch 5) is:
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
> 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 (clipcull_visitor.variable_found(2) &&
> + clipcull_visitor.variable_found(1)) {
> 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 (clipcull_visitor.variable_found(0)) {
> 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 (clipcull_visitor.variable_found(1)) {
> 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 +678,24 @@ 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");
> + const char *gl_Position = "gl_Position";
> + find_assignment_visitor find(1, &gl_Position);
> find.run(shader->ir);
> - if (!find.variable_found()) {
> + if (!find.variable_found(0)) {
> 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 +725,29 @@ 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");
> + static const char * const variable_names[] = {
> + "gl_FragColor",
> + "gl_FragData"
> + };
> + find_assignment_visitor frag_colordata(2, variable_names);
>
> - frag_color.run(shader->ir);
> - frag_data.run(shader->ir);
> + frag_colordata.run(shader->ir);
>
> - if (frag_color.variable_found() && frag_data.variable_found()) {
> + if (frag_colordata.variable_found(0) && frag_colordata.variable_found(1)) {
> 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.
More information about the mesa-dev
mailing list