On Mon, 2015-11-02 at 03:27 -0500, Ilia Mirkin wrote: > On Sun, Nov 1, 2015 at 3:33 AM, Timothy Arceri <t_arc...@yahoo.com.au> > wrote: > > Handles the case with function inout params where array elements > > do an assignment to themselves e.g. > > > > void array_mod(inout int b[2]) > > { > > b[0] = int(2); > > b[1] = b[1]; > > } > > > > Fixes assert in nir for: > > ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 > > > > https://bugs.freedesktop.org/show_bug.cgi?id=92588 > > --- > > src/glsl/opt_copy_propagation_elements.cpp | 46 > > ++++++++++++++++++++++++++++++ > > 1 file changed, 46 insertions(+) > > > > Piglit test: https://patchwork.freedesktop.org/patch/63355/ > > > > diff --git a/src/glsl/opt_copy_propagation_elements.cpp > > b/src/glsl/opt_copy_propagation_elements.cpp > > index 353a5c6..a62b625 100644 > > --- a/src/glsl/opt_copy_propagation_elements.cpp > > +++ b/src/glsl/opt_copy_propagation_elements.cpp > > @@ -439,6 +439,8 @@ ir_copy_propagation_elements_visitor::kill(kill_entry > > *k) > > /** > > * Adds directly-copied channels between vector variables to the > > available > > * copy propagation list. > > + * > > + * Also tidy up redundant function inout assignments while we are here. > > */ > > void > > ir_copy_propagation_elements_visitor::add_copy(ir_assignment *ir) > > @@ -450,6 +452,50 @@ > > ir_copy_propagation_elements_visitor::add_copy(ir_assignment *ir) > > if (ir->condition) > > return; > > > > + /* Handle a corner case with function inout params where array > > elements > > + * do an assignment to themselves e.g. > > + * > > + * void array_mod(inout int b[2]) > > + * { > > + * b[0] = int(2); > > + * b[1] = b[1]; > > + * } > > What if you have like > > array_mod(inout vec2 b[2]) { > b[1] = b[1].xy;
This is handled by the change also. The swizzle will be removed before getting here. > or > b[1] = b[1].yx; This will fail the first rhs_array->as_dereference_array() check (as it will be a swizzle) so we will leave it alone as we should. Just to be sure I wrote a couple of piglit tests and Cc'ed you in on them. > } > > IOW, why is this case so special? > > > + */ > > + ir_rvalue *rhs_array = ir->rhs; > > + ir_rvalue *lhs_array = ir->lhs; > > + if (lhs_array->as_dereference_array() && > > + rhs_array->as_dereference_array()) { > > + /* Check arrays are indexed by a const and match otherwise return > > */ > > + while (rhs_array->as_dereference_array() && > > + lhs_array->as_dereference_array()) { > > + > > + ir_dereference_array *rhs_deref_array = > > + rhs_array->as_dereference_array(); > > + ir_dereference_array *lhs_deref_array = > > + lhs_array->as_dereference_array(); > > + > > + ir_constant *lhs_ai_const = > > + lhs_deref_array->array_index->as_constant(); > > + ir_constant *rhs_ai_const = > > + rhs_deref_array->array_index->as_constant(); > > + if (lhs_ai_const == NULL || rhs_ai_const == NULL || > > + lhs_ai_const->value.i[0] != rhs_ai_const->value.i[0]) > > + return; > > + lhs_array = lhs_deref_array->array; > > + rhs_array = rhs_deref_array->array; > > + } > > + > > + ir_dereference_variable *lhs_var = lhs_array > > ->as_dereference_variable(); > > + ir_dereference_variable *rhs_var = rhs_array > > ->as_dereference_variable(); > > + if(lhs_var && rhs_var && lhs_var->var == rhs_var->var){ > > spaces please > > > + /* Removing the assignment now would mess up the loop iteration > > + * calling us. Just flag it to not execute, and someone else > > + * will clean up the mess. > > + */ > > + ir->condition = new(ralloc_parent(ir)) ir_constant(false); > > I thought that the allocator was smart and you could just do new (ir) > ir_constant(false). e.g. > > src/glsl/lower_jumps.cpp: new (ir) ir_constant(true))); Yeah I think your right, this was a copy and paste from opt_copy_propagation.cpp will chnage before pushing thanks. > > > + } > > + } > > + > > ir_dereference_variable *lhs = ir->lhs->as_dereference_variable(); > > if (!lhs || !(lhs->type->is_scalar() || lhs->type->is_vector())) > > return; > > -- > > 2.4.3 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev