Mesa (master): glsl: Remove field array_lvalue from ir_variable.

Paul Berry stereotype441 at kemper.freedesktop.org
Wed Sep 14 18:01:34 UTC 2011


Module: Mesa
Branch: master
Commit: 00792e3586746c833ffc9bb65712e38038916e06
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=00792e3586746c833ffc9bb65712e38038916e06

Author: Paul Berry <stereotype441 at gmail.com>
Date:   Sat Sep 10 07:48:46 2011 -0700

glsl: Remove field array_lvalue from ir_variable.

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.

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.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

---

 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..ce29d5a 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 cannot be out or inout parameters in GLSL 1.10");
+      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;




More information about the mesa-commit mailing list