-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 09/12/2011 06:20 PM, Paul Berry wrote:
> The array_lvalue field was attempting to enforce the restriction
> that whole arrays can't be used on the left-hand side of an
> assignment in GLSL 1.10 or GLSL ES, and can't be used as out or
> inout parameters in GLSL 1.10.
>
> However, it was buggy (it didn't work properly for built-in
> arrays), and it was clumsy (it unnecessarily kept track on a
> variable-by-variable basis, and it didn't cover the GLSL ES case).
>
> This patch removes the array_lvalue field completely in favor of
> explicit checks in ast_parameter_declarator::hir() (this check is
> added) and in do_assignment (this check was already present).
>
> This causes a benign behavioral change: when the user attempts to
> pass an array as an out or inout parameter of a function in GLSL
> 1.10, the error is now flagged at the time the function definition
> is encountered, rather than at the time of invocation. Previously
> we allowed such functions to be defined, and only flagged the error
> if they were invoked.
Other than the one comment below,
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
> Fixes Piglit tests
> spec/glsl-1.10/compiler/qualifiers/fn-{out,inout}-array-prohibited*
>
>
and
> spec/glsl-1.20/compiler/assignment-operators/assign-builtin-array-allowed.vert.
>
>
- ---
> src/glsl/ast_to_hir.cpp | 39
> ++++++++++++++++++++------------------- src/glsl/ir.cpp |
> 5 +---- src/glsl/ir.h | 8 --------
> src/glsl/ir_clone.cpp | 1 - 4 files changed, 21 insertions(+),
> 32 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 484786c..579426b 100644 --- a/src/glsl/ast_to_hir.cpp +++
> b/src/glsl/ast_to_hir.cpp @@ -2120,25 +2120,6 @@
> apply_type_qualifier_to_variable(const struct ast_type_qualifier
> *qual, var->depth_layout = ir_depth_layout_unchanged; else
> var->depth_layout = ir_depth_layout_none; - - /* From page 46
> (page 52 of the PDF) of the GLSL ES specification: - * - *
> "Array variables are l-values and may be passed to parameters -
> * declared as out or inout. However, they may not be used as -
> * the target of an assignment." - * - * From page 32
> (page 38 of the PDF) of the GLSL 1.10 spec: - * - * "Other
> binary or unary expressions, non-dereferenced arrays, - *
> function names, swizzles with repeated fields, and constants - *
> cannot be l-values." - * - * So we only mark 1.10 as
> non-lvalues, and check for array - * assignment in 100
> specifically in do_assignment. - */ - if
> (var->type->is_array() && state->language_version != 110) { -
> var->array_lvalue = true; - } }
>
> /** @@ -2953,6 +2934,26 @@ ast_parameter_declarator::hir(exec_list
> *instructions, type = glsl_type::error_type; }
>
> + /* From page 39 (page 45 of the PDF) of the GLSL 1.10 spec: +
> * + * "When calling a function, expressions that do not
> evaluate to + * l-values cannot be passed to parameters
> declared as out or inout." + * + * From page 32 (page 38 of
> the PDF) of the GLSL 1.10 spec: + * + * "Other binary or
> unary expressions, non-dereferenced arrays, + * function
> names, swizzles with repeated fields, and constants + *
> cannot be l-values." + * + * So for GLSL 1.10, passing an
> array as an out or inout parameter is not + * allowed. This
> restriction is removed in GLSL 1.20, and in GLSL ES. + */ + if
> ((var->mode == ir_var_inout || var->mode == ir_var_out) + &&
> type->is_array() && state->language_version == 110) { +
> _mesa_glsl_error(&loc, state, "Arrays are not l-values in GLSL
> 1.10");
The error message should say what the actual error is. While this
message is technically correct, it will confuse / irritate some
people. "Arrays cannot be out or inout parameters in GLSL 1.10" is
better.
> + type = glsl_type::error_type; + } +
> instructions->push_tail(var);
>
> /* Parameter declarations do not have r-values. diff --git
> a/src/glsl/ir.cpp b/src/glsl/ir.cpp index 41ed4f1..d6594cd 100644
> --- a/src/glsl/ir.cpp +++ b/src/glsl/ir.cpp @@ -1105,9 +1105,6 @@
> ir_dereference::is_lvalue() const if ((var == NULL) ||
> var->read_only) return false;
>
> - if (this->type->is_array() && !var->array_lvalue) - return
> false; - /* From page 17 (page 23 of the PDF) of the GLSL 1.20
> spec: * * "Samplers cannot be treated as l-values; hence cannot
> be used @@ -1323,7 +1320,7 @@ ir_swizzle::variable_referenced()
> const ir_variable::ir_variable(const struct glsl_type *type, const
> char *name, ir_variable_mode mode) : max_array_access(0),
> read_only(false), centroid(false), invariant(false), -
> mode(mode), interpolation(ir_var_smooth), array_lvalue(false) +
> mode(mode), interpolation(ir_var_smooth) { this->ir_type =
> ir_type_variable; this->type = type; diff --git a/src/glsl/ir.h
> b/src/glsl/ir.h index 2e899f3..cc4e680 100644 --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h @@ -346,14 +346,6 @@ public: unsigned
> interpolation:2;
>
> /** - * Flag that the whole array is assignable - * - * In
> GLSL 1.20 and later whole arrays are assignable (and comparable
> for - * equality). This flag enables this behavior. - */ -
> unsigned array_lvalue:1; - - /** * \name
> ARB_fragment_coord_conventions * @{ */ diff --git
> a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp index
> f075736..c1befa9 100644 --- a/src/glsl/ir_clone.cpp +++
> b/src/glsl/ir_clone.cpp @@ -47,7 +47,6 @@ ir_variable::clone(void
> *mem_ctx, struct hash_table *ht) const var->centroid =
> this->centroid; var->invariant = this->invariant;
> var->interpolation = this->interpolation; - var->array_lvalue =
> this->array_lvalue; var->location = this->location;
> var->warn_extension = this->warn_extension; var->origin_upper_left
> = this->origin_upper_left;
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk5vZe8ACgkQX1gOwKyEAw/7CwCdGruEaoHHvw24Yfx+x47GXzaw
HjQAnj9PKhN5oVUFPjLk3z8kjI0cP3cB
=NAXE
-----END PGP SIGNATURE-----
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
