Nicolai, this looks like a good candidate to nominate for inclusion in all the stable queues.
What do you think? On Tue, 2017-10-10 at 14:09 +0200, Nicolai Hähnle wrote: > From: Nicolai Hähnle <nicolai.haeh...@amd.com> > > The intended rule has been clarified in GLSL 4.60, Section 8.13.2 > (Interpolation Functions): > > "For all of the interpolation functions, interpolant must be an l-value > from an in declaration; this can include a variable, a block or > structure member, an array element, or some combination of these. > Component selection operators (e.g., .xy) may be used when specifying > interpolant." > > For members of interface blocks, var->data.must_be_shader_input must be > determined on-the-fly after lowering interface blocks, since we don't want > to disable varying packing for an entire block just because one input in it > is used in interpolateAt*. > > v2: keep setting must_be_shader_input in ast_function (Ian) > v3: follow the relaxed rule of GLSL 4.60 > v4: only apply the relaxed rules to desktop GL > (the ES WG decided that the relaxed rules may apply in a future version > but not retroactively; see also > > dEQP-GLES31.functional.shaders.multisample_interpolation.interpolate_at_centroid.negative.*) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101378 > Reviewed-by: Ian Romanick <ian.d.roman...@intel.com> (v1) > --- > src/compiler/glsl/ast_function.cpp | 19 ++++++++++++++----- > src/compiler/glsl/lower_named_interface_blocks.cpp | 18 ++++++++++++++++++ > 2 files changed, 32 insertions(+), 5 deletions(-) > > diff --git a/src/compiler/glsl/ast_function.cpp > b/src/compiler/glsl/ast_function.cpp > index 46a61e46fd5..d1596c272e6 100644 > --- a/src/compiler/glsl/ast_function.cpp > +++ b/src/compiler/glsl/ast_function.cpp > @@ -220,33 +220,42 @@ verify_parameter_modes(_mesa_glsl_parse_state *state, > if (val->ir_type == ir_type_swizzle) { > if (!state->is_version(440, 0)) { > _mesa_glsl_error(&loc, state, > "parameter `%s` must not be swizzled", > formal->name); > return false; > } > val = ((ir_swizzle *)val)->val; > } > > - while (val->ir_type == ir_type_dereference_array) { > - val = ((ir_dereference_array *)val)->array; > + for (;;) { > + if (val->ir_type == ir_type_dereference_array) { > + val = ((ir_dereference_array *)val)->array; > + } else if (val->ir_type == ir_type_dereference_record && > + !state->es_shader) { > + val = ((ir_dereference_record *)val)->record; > + } else > + break; > } > > - if (!val->as_dereference_variable() || > - val->variable_referenced()->data.mode != ir_var_shader_in) { > + ir_variable *var = NULL; > + if (const ir_dereference_variable *deref_var = > val->as_dereference_variable()) > + var = deref_var->variable_referenced(); > + > + if (!var || var->data.mode != ir_var_shader_in) { > _mesa_glsl_error(&loc, state, > "parameter `%s` must be a shader input", > formal->name); > return false; > } > > - val->variable_referenced()->data.must_be_shader_input = 1; > + var->data.must_be_shader_input = 1; > } > > /* Verify that 'out' and 'inout' actual parameters are lvalues. */ > if (formal->data.mode == ir_var_function_out > || formal->data.mode == ir_var_function_inout) { > const char *mode = NULL; > switch (formal->data.mode) { > case ir_var_function_out: mode = "out"; break; > case ir_var_function_inout: mode = "inout"; break; > default: assert(false); break; > diff --git a/src/compiler/glsl/lower_named_interface_blocks.cpp > b/src/compiler/glsl/lower_named_interface_blocks.cpp > index 064694128bf..136352a131b 100644 > --- a/src/compiler/glsl/lower_named_interface_blocks.cpp > +++ b/src/compiler/glsl/lower_named_interface_blocks.cpp > @@ -108,20 +108,21 @@ public: > > flatten_named_interface_blocks_declarations(void *mem_ctx) > : mem_ctx(mem_ctx), > interface_namespace(NULL) > { > } > > void run(exec_list *instructions); > > virtual ir_visitor_status visit_leave(ir_assignment *); > + virtual ir_visitor_status visit_leave(ir_expression *); > virtual void handle_rvalue(ir_rvalue **rvalue); > }; > > } /* anonymous namespace */ > > void > flatten_named_interface_blocks_declarations::run(exec_list *instructions) > { > interface_namespace = _mesa_hash_table_create(NULL, _mesa_key_hash_string, > _mesa_key_string_equal); > @@ -231,20 +232,37 @@ > flatten_named_interface_blocks_declarations::visit_leave(ir_assignment *ir) > } > > ir_variable *lhs_var = lhs_rec_tmp->variable_referenced(); > if (lhs_var) { > lhs_var->data.assigned = 1; > } > } > return rvalue_visit(ir); > } > > +ir_visitor_status > +flatten_named_interface_blocks_declarations::visit_leave(ir_expression *ir) > +{ > + ir_visitor_status status = rvalue_visit(ir); > + > + if (ir->operation == ir_unop_interpolate_at_centroid || > + ir->operation == ir_binop_interpolate_at_offset || > + ir->operation == ir_binop_interpolate_at_sample) { > + const ir_rvalue *val = ir->operands[0]; > + > + /* This disables varying packing for this input. */ > + val->variable_referenced()->data.must_be_shader_input = 1; > + } > + > + return status; > +} > + > void > flatten_named_interface_blocks_declarations::handle_rvalue(ir_rvalue > **rvalue) > { > if (*rvalue == NULL) > return; > > ir_dereference_record *ir = (*rvalue)->as_dereference_record(); > if (ir == NULL) > return; > -- Br, Andres _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev