From a61a0dbed29e284551ec7d033a0d2c8f2e1d6888 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Fri, 22 Mar 2013 17:49:27 -0700 Subject: [PATCH] glsl: Death to array dereferences of vectors! 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 Reviewed-by: Eric Anholt Reviewed-by: Kenneth Graunke --- 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 5db2576..ce96f685 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 5349e58..d5ad692 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; -- 2.7.4