Obligatory question: piglit tests? This series looks like a good solution for the problem in the bug, but I always worry that we've caught N-1 possible cases. That can be easier to detect from the set of tests.
If there are more problem found later, we can fix them later. This series is Reviewed-by: Ian Romanick <ian.d.roman...@intel.com> On 04/28/2016 11:06 PM, Kenneth Graunke wrote: > The old visitor missed some cases. For example, it wouldn't handle > an ir_dereference_array with a vector_extract as the index. > > Rather than trying to add the missing cases, just rewrite it as an > ir_rvalue_visitor. This makes it easy to replace any expression, > and is much less code. > > Cc: mesa-sta...@lists.freedesktop.org > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95164 > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/compiler/glsl/lower_vec_index_to_swizzle.cpp | 96 > ++++-------------------- > 1 file changed, 13 insertions(+), 83 deletions(-) > > diff --git a/src/compiler/glsl/lower_vec_index_to_swizzle.cpp > b/src/compiler/glsl/lower_vec_index_to_swizzle.cpp > index 8b18e95..b49255e 100644 > --- a/src/compiler/glsl/lower_vec_index_to_swizzle.cpp > +++ b/src/compiler/glsl/lower_vec_index_to_swizzle.cpp > @@ -30,18 +30,14 @@ > */ > > #include "ir.h" > -#include "ir_visitor.h" > +#include "ir_rvalue_visitor.h" > #include "ir_optimization.h" > #include "compiler/glsl_types.h" > #include "main/macros.h" > > -/** > - * Visitor class for replacing expressions with ir_constant values. > - */ > - > namespace { > > -class ir_vec_index_to_swizzle_visitor : public ir_hierarchical_visitor { > +class ir_vec_index_to_swizzle_visitor : public ir_rvalue_visitor { > public: > ir_vec_index_to_swizzle_visitor() > { > @@ -50,30 +46,28 @@ public: > > ir_rvalue *convert_vector_extract_to_swizzle(ir_rvalue *val); > > - virtual ir_visitor_status visit_enter(ir_expression *); > - virtual ir_visitor_status visit_enter(ir_swizzle *); > - virtual ir_visitor_status visit_enter(ir_assignment *); > - virtual ir_visitor_status visit_enter(ir_return *); > - virtual ir_visitor_status visit_enter(ir_call *); > - virtual ir_visitor_status visit_enter(ir_if *); > + virtual void handle_rvalue(ir_rvalue **); > > bool progress; > }; > > } /* anonymous namespace */ > > -ir_rvalue * > -ir_vec_index_to_swizzle_visitor::convert_vector_extract_to_swizzle(ir_rvalue > *ir) > +void > +ir_vec_index_to_swizzle_visitor::handle_rvalue(ir_rvalue **rv) > { > - ir_expression *const expr = ir->as_expression(); > + if (*rv == NULL) > + return; > + > + ir_expression *const expr = (*rv)->as_expression(); > if (expr == NULL || expr->operation != ir_binop_vector_extract) > - return ir; > + return; > > ir_constant *const idx = expr->operands[1]->constant_expression_value(); > if (idx == NULL) > - return ir; > + return; > > - void *ctx = ralloc_parent(ir); > + void *ctx = ralloc_parent(expr); > this->progress = true; > > /* Page 40 of the GLSL 1.20 spec says: > @@ -93,71 +87,7 @@ > ir_vec_index_to_swizzle_visitor::convert_vector_extract_to_swizzle(ir_rvalue > *ir > const int i = CLAMP(idx->value.i[0], 0, > (int) expr->operands[0]->type->vector_elements - 1); > > - return new(ctx) ir_swizzle(expr->operands[0], i, 0, 0, 0, 1); > -} > - > -ir_visitor_status > -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_vector_extract_to_swizzle(ir->operands[i]); > - } > - > - return visit_continue; > -} > - > -ir_visitor_status > -ir_vec_index_to_swizzle_visitor::visit_enter(ir_swizzle *ir) > -{ > - /* Can't be hit from normal GLSL, since you can't swizzle a scalar (which > - * 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_vector_extract_to_swizzle(ir->val); > - > - return visit_continue; > -} > - > -ir_visitor_status > -ir_vec_index_to_swizzle_visitor::visit_enter(ir_assignment *ir) > -{ > - ir->rhs = convert_vector_extract_to_swizzle(ir->rhs); > - > - return visit_continue; > -} > - > -ir_visitor_status > -ir_vec_index_to_swizzle_visitor::visit_enter(ir_call *ir) > -{ > - foreach_in_list_safe(ir_rvalue, param, &ir->actual_parameters) { > - ir_rvalue *new_param = convert_vector_extract_to_swizzle(param); > - > - if (new_param != param) { > - param->replace_with(new_param); > - } > - } > - > - return visit_continue; > -} > - > -ir_visitor_status > -ir_vec_index_to_swizzle_visitor::visit_enter(ir_return *ir) > -{ > - if (ir->value) { > - ir->value = convert_vector_extract_to_swizzle(ir->value); > - } > - > - return visit_continue; > -} > - > -ir_visitor_status > -ir_vec_index_to_swizzle_visitor::visit_enter(ir_if *ir) > -{ > - ir->condition = convert_vector_extract_to_swizzle(ir->condition); > - > - return visit_continue; > + *rv = new(ctx) ir_swizzle(expr->operands[0], i, 0, 0, 0, 1); > } > > bool > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
