[Mesa-dev] [PATCH 3/9] glsl: look for multiple variables simultaneously with find_assignment_visitor
Timothy Arceri
tarceri at itsqueeze.com
Mon Jun 26 13:18:02 UTC 2017
On 26/06/17 20:28, Nicolai Hähnle wrote:
> On 26.06.2017 12:18, Timothy Arceri wrote:
>> 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.
>
> Yeah, I tend to agree and wasn't certain about this patch. I'll think
> about it some more, and depending on if I come up with something I
> might just drop it.
Probably overkill but you could use string_to_uint_map.
>
>
>> 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>
>
> Thanks!
> Nicolai
>
>
>>
>>> 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