glsl: make max array trackers ints and use -1 as base. (v2)

This fixes a bug that breaks cull distances. The problem
is the max array accessors can't tell the difference between
an never accessed unsized array and an accessed at location 0
unsized array. This leads to converting an undeclared unused
gl_ClipDistance inside or outside gl_PerVertex to a size 1
array. However we need to the number of active clip distances
to work out the starting point for the cull distances, and
this offset by one when it's not being used isn't possible
to distinguish from the case were only the first element is
accessed. I tried to use ->used for this, but that doesn't
work when gl_ClipDistance is part of an interface block.

So this changes things so that max_array_access is an int
and initialised to -1. This also allows unsized arrays to
proceed further than that could before, but we really shouldn't
mind as they will get eliminated if nothing uses them later.

For initialised uniforms we no longer change their array
size at runtime, if these are unused they will get eliminated
eventually.

v2: use ralloc_array (Ilia)

Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Dave Airlie <airlied@redhat.com>

diff --git a/src/compiler/glsl/ast_array_index.cpp b/src/compiler/glsl/ast_array_index.cpp
index 69322cf111f35827ef30accba187e13961f99db2..2e36035f9f427218cda4f1b7ac0d020aab232094 100644 (file)
--- a/src/compiler/glsl/ast_array_index.cpp
+++ b/src/compiler/glsl/ast_array_index.cpp
@@ -92,12 +92,12 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
                deref_record->record->type->field_index(deref_record->field);
             assert(field_index < deref_var->var->get_interface_type()->length);
 
-            unsigned *const max_ifc_array_access =
+            int *const max_ifc_array_access =
                deref_var->var->get_max_ifc_array_access();
 
             assert(max_ifc_array_access != NULL);
 
-            if (idx > (int)max_ifc_array_access[field_index]) {
+            if (idx > max_ifc_array_access[field_index]) {
                max_ifc_array_access[field_index] = idx;
 
                /* Check whether this access will, as a side effect, implicitly

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index ecd1327e358190d74c45616625b508b4d3c99ad7..1ddb2bf43dbcdf318cf934aa3633dc429c212e38 100644 (file)
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -976,7 +976,7 @@ do_assignment(exec_list *instructions, struct _mesa_glsl_parse_state *state,
 
          assert(var != NULL);
 
-         if (var->data.max_array_access >= unsigned(rhs->type->array_size())) {
+         if (var->data.max_array_access >= rhs->type->array_size()) {
             /* FINISHME: This should actually log the location of the RHS. */
             _mesa_glsl_error(& lhs_loc, state, "array size must be > %u due to "
                              "previous access",
@@ -3866,7 +3866,7 @@ get_variable_being_redeclared(ir_variable *var, YYLTYPE loc,
        * FINISHME: required or not.
        */
 
-      const unsigned size = unsigned(var->type->array_size());
+      const int size = var->type->array_size();
       check_builtin_array_max_size(var->name, size, loc, state);
       if ((size > 0) && (size <= earlier->data.max_array_access)) {
          _mesa_glsl_error(& loc, state, "array size must be > %u due to "
@@ -7737,7 +7737,7 @@ ast_tcs_output_layout::hir(exec_list *instructions,
       if (!var->type->is_unsized_array() || var->data.patch)
          continue;
 
-      if (var->data.max_array_access >= num_vertices) {
+      if (var->data.max_array_access >= (int)num_vertices) {
         _mesa_glsl_error(&loc, state,
                          "this tessellation control shader output layout "
                          "specifies %u vertices, but an access to element "
@@ -7798,7 +7798,7 @@ ast_gs_input_layout::hir(exec_list *instructions,
        */
 
       if (var->type->is_unsized_array()) {
-         if (var->data.max_array_access >= num_vertices) {
+         if (var->data.max_array_access >= (int)num_vertices) {
             _mesa_glsl_error(&loc, state,
                              "this geometry shader input layout implies %u"
                              " vertices, but an access to element %u of input"

diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 9637d7ad78cc42f2b1d13ce8ff8c26548b1b8b6e..5bb3ac3c214f8e7caf12153378bca34eb083a212 100644 (file)
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -1668,7 +1668,7 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name,
    this->data.how_declared = ir_var_declared_normally;
    this->data.mode = mode;
    this->data.interpolation = INTERP_QUALIFIER_NONE;
-   this->data.max_array_access = 0;
+   this->data.max_array_access = -1;
    this->data.offset = 0;
    this->data.precision = GLSL_PRECISION_NONE;
    this->data.image_read_only = false;

diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index 6e0dc0b1bcdd1edf5bc7e74e926633b34406d979..d52dbf8a38022e972c0f6a7c4b6c6c0329cafd2d 100644 (file)
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -484,7 +484,10 @@ public:
       this->interface_type = type;
       if (this->is_interface_instance()) {
          this->u.max_ifc_array_access =
-            rzalloc_array(this, unsigned, type->length);
+            ralloc_array(this, int, type->length);
+         for (unsigned i = 0; i < type->length; i++) {
+            this->u.max_ifc_array_access[i] = -1;
+         }
       }
    }
 
@@ -520,7 +523,7 @@ public:
           * zero.
           */
          for (unsigned i = 0; i < this->interface_type->length; i++)
-            assert(this->u.max_ifc_array_access[i] == 0);
+            assert(this->u.max_ifc_array_access[i] == -1);
 #endif
          ralloc_free(this->u.max_ifc_array_access);
          this->u.max_ifc_array_access = NULL;
@@ -540,7 +543,7 @@ public:
     * A "set" function is not needed because the array is dynmically allocated
     * as necessary.
     */
-   inline unsigned *get_max_ifc_array_access()
+   inline int *get_max_ifc_array_access()
    {
       assert(this->data._num_state_slots == 0);
       return this->u.max_ifc_array_access;
@@ -888,9 +891,9 @@ public:
       /**
        * Highest element accessed with a constant expression array index
        *
-       * Not used for non-array variables.
+       * Not used for non-array variables. -1 is never accessed.
        */
-      unsigned max_array_access;
+      int max_array_access;
 
       /**
        * Transform feedback buffer.
@@ -938,7 +941,7 @@ private:
        * For variables whose type is not an interface block, this pointer is
        * NULL.
        */
-      unsigned *max_ifc_array_access;
+      int *max_ifc_array_access;
 
       /**
        * Built-in state that backs this uniform

diff --git a/src/compiler/glsl/ir_clone.cpp b/src/compiler/glsl/ir_clone.cpp
index 43ffffb0a38defc12943b732620a7f015021207e..60d15261275a7157e963b794b1f637ff8e2d109c 100644 (file)
--- a/src/compiler/glsl/ir_clone.cpp
+++ b/src/compiler/glsl/ir_clone.cpp
@@ -46,7 +46,7 @@ ir_variable::clone(void *mem_ctx, struct hash_table *ht) const
    var->data.max_array_access = this->data.max_array_access;
    if (this->is_interface_instance()) {
       var->u.max_ifc_array_access =
-         rzalloc_array(var, unsigned, this->interface_type->length);
+         rzalloc_array(var, int, this->interface_type->length);
       memcpy(var->u.max_ifc_array_access, this->u.max_ifc_array_access,
              this->interface_type->length * sizeof(unsigned));
    }

diff --git a/src/compiler/glsl/ir_validate.cpp b/src/compiler/glsl/ir_validate.cpp
index 2ec5a3f73f73adcca33bbfe6db354e3eaf5a3fcd..9d05e7e00a9f986c07c7ea0b190f991d7c377546 100644 (file)
--- a/src/compiler/glsl/ir_validate.cpp
+++ b/src/compiler/glsl/ir_validate.cpp
@@ -727,7 +727,7 @@ ir_validate::visit(ir_variable *ir)
     * to be out of bounds.
     */
    if (ir->type->array_size() > 0) {
-      if (ir->data.max_array_access >= ir->type->length) {
+      if (ir->data.max_array_access >= (int)ir->type->length) {
         printf("ir_variable has maximum access out of bounds (%d vs %d)\n",
                ir->data.max_array_access, ir->type->length - 1);
         ir->print();
@@ -744,12 +744,12 @@ ir_validate::visit(ir_variable *ir)
          ir->get_interface_type()->fields.structure;
       for (unsigned i = 0; i < ir->get_interface_type()->length; i++) {
          if (fields[i].type->array_size() > 0) {
-            const unsigned *const max_ifc_array_access =
+            const int *const max_ifc_array_access =
                ir->get_max_ifc_array_access();
 
             assert(max_ifc_array_access != NULL);
 
-            if (max_ifc_array_access[i] >= fields[i].type->length) {
+            if (max_ifc_array_access[i] >= (int)fields[i].type->length) {
                printf("ir_variable has maximum access out of bounds for "
                       "field %s (%d vs %d)\n", fields[i].name,
                       max_ifc_array_access[i], fields[i].type->length);

diff --git a/src/compiler/glsl/link_functions.cpp b/src/compiler/glsl/link_functions.cpp
index 537f4dc77ac7532c50bf48b0ebd34cfb2ec02a3b..4e1028769fe3b0dfe60b86eecbec22c198693b22 100644 (file)
--- a/src/compiler/glsl/link_functions.cpp
+++ b/src/compiler/glsl/link_functions.cpp
@@ -245,9 +245,9 @@ public:
                /* Similarly, we need implicit sizes of arrays within interface
                 * blocks to be sized by the maximal access in *any* shader.
                 */
-               unsigned *const linked_max_ifc_array_access =
+               int *const linked_max_ifc_array_access =
                   var->get_max_ifc_array_access();
-               unsigned *const ir_max_ifc_array_access =
+               int *const ir_max_ifc_array_access =
                   ir->var->get_max_ifc_array_access();
 
                assert(linked_max_ifc_array_access != NULL);

diff --git a/src/compiler/glsl/link_interface_blocks.cpp b/src/compiler/glsl/link_interface_blocks.cpp
index 26072591b0e4eb444c9b2a9f8fd3d165c91c1875..4eda09774efc5d8c78ccff63b942d074314b496c 100644 (file)
--- a/src/compiler/glsl/link_interface_blocks.cpp
+++ b/src/compiler/glsl/link_interface_blocks.cpp
@@ -154,12 +154,6 @@ static bool
 interstage_match(struct gl_shader_program *prog, ir_variable *producer,
                  ir_variable *consumer, bool extra_array_level)
 {
-   /* Unsized arrays should not occur during interstage linking.  They
-    * should have all been assigned a size by link_intrastage_shaders.
-    */
-   assert(!consumer->type->is_unsized_array());
-   assert(!producer->type->is_unsized_array());
-
    /* Types must match. */
    if (consumer->get_interface_type() != producer->get_interface_type()) {
       /* Exception: if both the interface blocks are implicitly declared,

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 5e59ae3e5b21dd0aacaf34951406080da0cc18d7..f4d443b3f8e4373b074d5ee3583273c4cd7d2a1c 100644 (file)
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -216,7 +216,7 @@ public:
        * array using an index too large for its actual size assigned at link
        * time.
        */
-      if (var->data.max_array_access >= this->num_vertices) {
+      if (var->data.max_array_access >= (int)this->num_vertices) {
          linker_error(this->prog, "geometry shader accesses element %i of "
                       "%s, but only %i input vertices\n",
                       var->data.max_array_access, var->name, this->num_vertices);
@@ -924,7 +924,7 @@ validate_intrastage_arrays(struct gl_shader_program *prog,
       if ((var->type->fields.array == existing->type->fields.array) &&
           ((var->type->length == 0)|| (existing->type->length == 0))) {
          if (var->type->length != 0) {
-            if (var->type->length <= existing->data.max_array_access) {
+            if ((int)var->type->length <= existing->data.max_array_access) {
                linker_error(prog, "%s `%s' declared as type "
                            "`%s' but outermost dimension has an index"
                            " of `%i'\n",
@@ -935,7 +935,7 @@ validate_intrastage_arrays(struct gl_shader_program *prog,
             existing->type = var->type;
             return true;
          } else if (existing->type->length != 0) {
-            if(existing->type->length <= var->data.max_array_access &&
+            if((int)existing->type->length <= var->data.max_array_access &&
                !existing->data.from_ssbo_unsized_array) {
                linker_error(prog, "%s `%s' declared as type "
                            "`%s' but outermost dimension has an index"
@@ -1593,7 +1593,7 @@ private:
     */
    static const glsl_type *
    resize_interface_members(const glsl_type *type,
-                            const unsigned *max_ifc_array_access,
+                            const int *max_ifc_array_access,
                             bool is_ssbo)
    {
       unsigned num_fields = type->length;
@@ -2399,10 +2399,10 @@ update_array_sizes(struct gl_shader_program *prog)
           * Subroutine uniforms are not removed.
          */
         if (var->is_in_buffer_block() || var->type->contains_atomic() ||
-            var->type->contains_subroutine())
+            var->type->contains_subroutine() || var->constant_initializer)
            continue;
 
-        unsigned int size = var->data.max_array_access;
+        int size = var->data.max_array_access;
         for (unsigned j = 0; j < MESA_SHADER_STAGES; j++) {
               if (prog->_LinkedShaders[j] == NULL)
                  continue;
@@ -2419,7 +2419,7 @@ update_array_sizes(struct gl_shader_program *prog)
            }
         }
 
-        if (size + 1 != var->type->length) {
+        if (size + 1 != (int)var->type->length) {
            /* If this is a built-in uniform (i.e., it's backed by some
             * fixed-function state), adjust the number of state slots to
             * match the new array size.  The number of slots per array entry

diff --git a/src/mesa/main/ff_fragment_shader.cpp b/src/mesa/main/ff_fragment_shader.cpp
index b0ce8c472efced10b52bc6e6f005b1dc02fb37f4..26bf162587895415bdb120e1fec6638fc14348fb 100644 (file)
--- a/src/mesa/main/ff_fragment_shader.cpp
+++ b/src/mesa/main/ff_fragment_shader.cpp
@@ -517,7 +517,7 @@ get_current_attrib(texenv_fragment_program *p, GLuint attrib)
 
    current = p->shader->symbols->get_variable("gl_CurrentAttribFragMESA");
    assert(current);
-   current->data.max_array_access = MAX2(current->data.max_array_access, attrib);
+   current->data.max_array_access = MAX2(current->data.max_array_access, (int)attrib);
    val = new(p->mem_ctx) ir_dereference_variable(current);
    ir_rvalue *index = new(p->mem_ctx) ir_constant(attrib);
    return new(p->mem_ctx) ir_dereference_array(val, index);
@@ -561,7 +561,7 @@ get_source(texenv_fragment_program *p,
       var = p->shader->symbols->get_variable("gl_TextureEnvColor");
       assert(var);
       deref = new(p->mem_ctx) ir_dereference_variable(var);
-      var->data.max_array_access = MAX2(var->data.max_array_access, unit);
+      var->data.max_array_access = MAX2(var->data.max_array_access, (int)unit);
       return new(p->mem_ctx) ir_dereference_array(deref,
                                                  new(p->mem_ctx) ir_constant(unit));
 
@@ -893,7 +893,7 @@ static void load_texture( texenv_fragment_program *p, GLuint unit )
       texcoord = new(p->mem_ctx) ir_dereference_variable(tc_array);
       ir_rvalue *index = new(p->mem_ctx) ir_constant(unit);
       texcoord = new(p->mem_ctx) ir_dereference_array(texcoord, index);
-      tc_array->data.max_array_access = MAX2(tc_array->data.max_array_access, unit);
+      tc_array->data.max_array_access = MAX2(tc_array->data.max_array_access, (int)unit);
    }
 
    if (!p->state->unit[unit].enabled) {