[Mesa-dev] [PATCH 12/12] glsl: Death to array dereferences of vectors!

Ian Romanick idr at freedesktop.org
Fri May 3 16:07:54 PDT 2013


From: Ian Romanick <ian.d.romanick at intel.com>

Now that all the places that used to generate array derefeneces of
vectors have been changed to generate either ir_binop_vector_extract or
ir_triop_vector_insert (or both), remove all support for dealing with
this deprecated construct.

As an added safeguard, modify ir_validate to reject ir_dereference_array
of a vector.

v2: Convert tabs to spaces.  Suggested by Eric.

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
Reviewed-by: Eric Anholt <eric at anholt.net>
---
 src/glsl/ir_validate.cpp                    |  29 +++++++
 src/glsl/lower_vec_index_to_cond_assign.cpp | 116 +---------------------------
 src/glsl/lower_vec_index_to_swizzle.cpp     |  56 +-------------
 3 files changed, 32 insertions(+), 169 deletions(-)

diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
index 95bb0fe..52cfd39 100644
--- a/src/glsl/ir_validate.cpp
+++ b/src/glsl/ir_validate.cpp
@@ -69,6 +69,8 @@ public:
    virtual ir_visitor_status visit_leave(ir_expression *ir);
    virtual ir_visitor_status visit_leave(ir_swizzle *ir);
 
+   virtual ir_visitor_status visit_enter(class ir_dereference_array *);
+
    virtual ir_visitor_status visit_enter(ir_assignment *ir);
    virtual ir_visitor_status visit_enter(ir_call *ir);
 
@@ -102,6 +104,33 @@ ir_validate::visit(ir_dereference_variable *ir)
 }
 
 ir_visitor_status
+ir_validate::visit_enter(class ir_dereference_array *ir)
+{
+   if (!ir->array->type->is_array() && !ir->array->type->is_matrix()) {
+      printf("ir_dereference_array @ %p does not specify an array or a "
+             "matrix\n",
+             (void *) ir);
+      ir->print();
+      printf("\n");
+      abort();
+   }
+
+   if (!ir->array_index->type->is_scalar()) {
+      printf("ir_dereference_array @ %p does not have scalar index: %s\n",
+             (void *) ir, ir->array_index->type->name);
+      abort();
+   }
+
+   if (!ir->array_index->type->is_integer()) {
+      printf("ir_dereference_array @ %p does not have integer index: %s\n",
+             (void *) ir, ir->array_index->type->name);
+      abort();
+   }
+
+   return visit_continue;
+}
+
+ir_visitor_status
 ir_validate::visit_enter(ir_if *ir)
 {
    if (ir->condition->type != glsl_type::bool_type) {
diff --git a/src/glsl/lower_vec_index_to_cond_assign.cpp b/src/glsl/lower_vec_index_to_cond_assign.cpp
index 9d248ca..8808596 100644
--- a/src/glsl/lower_vec_index_to_cond_assign.cpp
+++ b/src/glsl/lower_vec_index_to_cond_assign.cpp
@@ -52,7 +52,6 @@ public:
       progress = false;
    }
 
-   ir_rvalue *convert_vec_index_to_cond_assign(ir_rvalue *val);
    ir_rvalue *convert_vec_index_to_cond_assign(void *mem_ctx,
                                                ir_rvalue *orig_vector,
                                                ir_rvalue *orig_index,
@@ -142,26 +141,6 @@ ir_vec_index_to_cond_assign_visitor::convert_vec_index_to_cond_assign(void *mem_
 }
 
 ir_rvalue *
-ir_vec_index_to_cond_assign_visitor::convert_vec_index_to_cond_assign(ir_rvalue *ir)
-{
-   ir_dereference_array *orig_deref = ir->as_dereference_array();
-
-   if (!orig_deref)
-      return ir;
-
-   if (orig_deref->array->type->is_matrix() ||
-       orig_deref->array->type->is_array())
-      return ir;
-
-   assert(orig_deref->array_index->type->base_type == GLSL_TYPE_INT);
-
-   return convert_vec_index_to_cond_assign(ralloc_parent(ir),
-                                           orig_deref->array,
-                                           orig_deref->array_index,
-                                           ir->type);
-}
-
-ir_rvalue *
 ir_vec_index_to_cond_assign_visitor::convert_vector_extract_to_cond_assign(ir_rvalue *ir)
 {
    ir_expression *const expr = ir->as_expression();
@@ -181,7 +160,6 @@ ir_vec_index_to_cond_assign_visitor::visit_enter(ir_expression *ir)
    unsigned int i;
 
    for (i = 0; i < ir->get_num_operands(); i++) {
-      ir->operands[i] = convert_vec_index_to_cond_assign(ir->operands[i]);
       ir->operands[i] = convert_vector_extract_to_cond_assign(ir->operands[i]);
    }
 
@@ -195,7 +173,6 @@ ir_vec_index_to_cond_assign_visitor::visit_enter(ir_swizzle *ir)
     * the result of indexing a vector is.  But maybe at some point we'll end up
     * using swizzling of scalars for vector construction.
     */
-   ir->val = convert_vec_index_to_cond_assign(ir->val);
    ir->val = convert_vector_extract_to_cond_assign(ir->val);
 
    return visit_continue;
@@ -204,95 +181,12 @@ ir_vec_index_to_cond_assign_visitor::visit_enter(ir_swizzle *ir)
 ir_visitor_status
 ir_vec_index_to_cond_assign_visitor::visit_leave(ir_assignment *ir)
 {
-   ir_variable *index, *var;
-   ir_dereference_variable *deref;
-   ir_assignment *assign;
-   unsigned i;
-
-   ir->rhs = convert_vec_index_to_cond_assign(ir->rhs);
    ir->rhs = convert_vector_extract_to_cond_assign(ir->rhs);
 
    if (ir->condition) {
-      ir->condition = convert_vec_index_to_cond_assign(ir->condition);
       ir->condition = convert_vector_extract_to_cond_assign(ir->condition);
    }
 
-   /* Last, handle the LHS */
-   ir_dereference_array *orig_deref = ir->lhs->as_dereference_array();
-
-   if (!orig_deref ||
-       orig_deref->array->type->is_matrix() ||
-       orig_deref->array->type->is_array())
-      return visit_continue;
-
-   void *mem_ctx = ralloc_parent(ir);
-
-   assert(orig_deref->array_index->type->base_type == GLSL_TYPE_INT);
-
-   exec_list list;
-
-   /* Store the index to a temporary to avoid reusing its tree. */
-   index = new(ir) ir_variable(glsl_type::int_type, "vec_index_tmp_i",
-			       ir_var_temporary);
-   list.push_tail(index);
-   deref = new(ir) ir_dereference_variable(index);
-   assign = new(ir) ir_assignment(deref, orig_deref->array_index, NULL);
-   list.push_tail(assign);
-
-   /* Store the RHS to a temporary to avoid reusing its tree. */
-   var = new(ir) ir_variable(ir->rhs->type, "vec_index_tmp_v",
-			     ir_var_temporary);
-   list.push_tail(var);
-   deref = new(ir) ir_dereference_variable(var);
-   assign = new(ir) ir_assignment(deref, ir->rhs, NULL);
-   list.push_tail(assign);
-
-   /* Generate a single comparison condition "mask" for all of the components
-    * in the vector.
-    */
-   ir_rvalue *const cond_deref =
-      compare_index_block(&list, index, 0,
-			  orig_deref->array->type->vector_elements,
-			  mem_ctx);
-
-   /* Generate a conditional move of each vector element to the temp. */
-   for (i = 0; i < orig_deref->array->type->vector_elements; i++) {
-      ir_rvalue *condition_swizzle =
-	 new(ir) ir_swizzle(cond_deref->clone(ir, NULL), i, 0, 0, 0, 1);
-
-
-      /* Just clone the rest of the deref chain when trying to get at the
-       * underlying variable.
-       */
-      ir_rvalue *swizzle =
-	 new(ir) ir_swizzle(orig_deref->array->clone(mem_ctx, NULL),
-			    i, 0, 0, 0, 1);
-
-      deref = new(ir) ir_dereference_variable(var);
-      assign = new(ir) ir_assignment(swizzle, deref, condition_swizzle);
-      list.push_tail(assign);
-   }
-
-   /* If the original assignment has a condition, respect that original
-    * condition!  This is acomplished by wrapping the new conditional
-    * assignments in an if-statement that uses the original condition.
-    */
-   if (ir->condition != NULL) {
-      /* No need to clone the condition because the IR that it hangs on is
-       * going to be removed from the instruction sequence.
-       */
-      ir_if *if_stmt = new(mem_ctx) ir_if(ir->condition);
-
-      list.move_nodes_to(&if_stmt->then_instructions);
-      ir->insert_before(if_stmt);
-   } else {
-      ir->insert_before(&list);
-   }
-
-   ir->remove();
-
-   this->progress = true;
-
    return visit_continue;
 }
 
@@ -301,16 +195,10 @@ ir_vec_index_to_cond_assign_visitor::visit_enter(ir_call *ir)
 {
    foreach_iter(exec_list_iterator, iter, *ir) {
       ir_rvalue *param = (ir_rvalue *)iter.get();
-      ir_rvalue *new_param = convert_vec_index_to_cond_assign(param);
+      ir_rvalue *new_param = convert_vector_extract_to_cond_assign(param);
 
       if (new_param != param) {
 	 param->replace_with(new_param);
-      } else {
-         new_param = convert_vector_extract_to_cond_assign(param);
-
-         if (new_param != param) {
-            param->replace_with(new_param);
-         }
       }
    }
 
@@ -321,7 +209,6 @@ ir_visitor_status
 ir_vec_index_to_cond_assign_visitor::visit_enter(ir_return *ir)
 {
    if (ir->value) {
-      ir->value = convert_vec_index_to_cond_assign(ir->value);
       ir->value = convert_vector_extract_to_cond_assign(ir->value);
    }
 
@@ -331,7 +218,6 @@ ir_vec_index_to_cond_assign_visitor::visit_enter(ir_return *ir)
 ir_visitor_status
 ir_vec_index_to_cond_assign_visitor::visit_enter(ir_if *ir)
 {
-   ir->condition = convert_vec_index_to_cond_assign(ir->condition);
    ir->condition = convert_vector_extract_to_cond_assign(ir->condition);
 
    return visit_continue;
diff --git a/src/glsl/lower_vec_index_to_swizzle.cpp b/src/glsl/lower_vec_index_to_swizzle.cpp
index 9d0d696..617e996 100644
--- a/src/glsl/lower_vec_index_to_swizzle.cpp
+++ b/src/glsl/lower_vec_index_to_swizzle.cpp
@@ -46,7 +46,6 @@ public:
       progress = false;
    }
 
-   ir_rvalue *convert_vec_index_to_swizzle(ir_rvalue *val);
    ir_rvalue *convert_vector_extract_to_swizzle(ir_rvalue *val);
 
    virtual ir_visitor_status visit_enter(ir_expression *);
@@ -60,46 +59,6 @@ public:
 };
 
 ir_rvalue *
-ir_vec_index_to_swizzle_visitor::convert_vec_index_to_swizzle(ir_rvalue *ir)
-{
-   ir_dereference_array *deref = ir->as_dereference_array();
-   ir_constant *ir_constant;
-
-   if (!deref)
-      return ir;
-
-   if (deref->array->type->is_matrix() || deref->array->type->is_array())
-      return ir;
-
-   assert(deref->array_index->type->base_type == GLSL_TYPE_INT);
-   ir_constant = deref->array_index->constant_expression_value();
-   if (!ir_constant)
-      return ir;
-
-   void *ctx = ralloc_parent(ir);
-   this->progress = true;
-
-   /* Page 40 of the GLSL 1.20 spec says:
-    *
-    *     "When indexing with non-constant expressions, behavior is undefined
-    *     if the index is negative, or greater than or equal to the size of
-    *     the vector."
-    *
-    * The quoted spec text mentions non-constant expressions, but this code
-    * operates on constants.  These constants are the result of non-constant
-    * expressions that have been optimized to constants.  The common case here
-    * is a loop counter from an unrolled loop that is used to index a vector.
-    *
-    * The ir_swizzle constructor gets angry if the index is negative or too
-    * large.  For simplicity sake, just clamp the index to [0, size-1].
-    */
-   const int i = MIN2(MAX2(ir_constant->value.i[0], 0),
-                      ((int) deref->array->type->vector_elements - 1));
-
-   return new(ctx) ir_swizzle(deref->array, i, 0, 0, 0, 1);
-}
-
-ir_rvalue *
 ir_vec_index_to_swizzle_visitor::convert_vector_extract_to_swizzle(ir_rvalue *ir)
 {
    ir_expression *const expr = ir->as_expression();
@@ -139,7 +98,6 @@ ir_vec_index_to_swizzle_visitor::visit_enter(ir_expression *ir)
    unsigned int i;
 
    for (i = 0; i < ir->get_num_operands(); i++) {
-      ir->operands[i] = convert_vec_index_to_swizzle(ir->operands[i]);
       ir->operands[i] = convert_vector_extract_to_swizzle(ir->operands[i]);
    }
 
@@ -153,7 +111,7 @@ ir_vec_index_to_swizzle_visitor::visit_enter(ir_swizzle *ir)
     * the result of indexing a vector is.  But maybe at some point we'll end up
     * using swizzling of scalars for vector construction.
     */
-   ir->val = convert_vec_index_to_swizzle(ir->val);
+   ir->val = convert_vector_extract_to_swizzle(ir->val);
 
    return visit_continue;
 }
@@ -161,8 +119,6 @@ ir_vec_index_to_swizzle_visitor::visit_enter(ir_swizzle *ir)
 ir_visitor_status
 ir_vec_index_to_swizzle_visitor::visit_enter(ir_assignment *ir)
 {
-   ir->set_lhs(convert_vec_index_to_swizzle(ir->lhs));
-   ir->rhs = convert_vec_index_to_swizzle(ir->rhs);
    ir->rhs = convert_vector_extract_to_swizzle(ir->rhs);
 
    return visit_continue;
@@ -173,16 +129,10 @@ ir_vec_index_to_swizzle_visitor::visit_enter(ir_call *ir)
 {
    foreach_iter(exec_list_iterator, iter, *ir) {
       ir_rvalue *param = (ir_rvalue *)iter.get();
-      ir_rvalue *new_param = convert_vec_index_to_swizzle(param);
+      ir_rvalue *new_param = convert_vector_extract_to_swizzle(param);
 
       if (new_param != param) {
 	 param->replace_with(new_param);
-      } else {
-         new_param = convert_vector_extract_to_swizzle(param);
-
-         if (new_param != param) {
-            param->replace_with(new_param);
-         }
       }
    }
 
@@ -193,7 +143,6 @@ ir_visitor_status
 ir_vec_index_to_swizzle_visitor::visit_enter(ir_return *ir)
 {
    if (ir->value) {
-      ir->value = convert_vec_index_to_swizzle(ir->value);
       ir->value = convert_vector_extract_to_swizzle(ir->value);
    }
 
@@ -203,7 +152,6 @@ ir_vec_index_to_swizzle_visitor::visit_enter(ir_return *ir)
 ir_visitor_status
 ir_vec_index_to_swizzle_visitor::visit_enter(ir_if *ir)
 {
-   ir->condition = convert_vec_index_to_swizzle(ir->condition);
    ir->condition = convert_vector_extract_to_swizzle(ir->condition);
 
    return visit_continue;
-- 
1.8.1.4



More information about the mesa-dev mailing list