On 01/11/2014 02:37 AM, Kenneth Graunke wrote: > When handling function calls, we often want to walk through the list of > formal parameters and list of actual parameters at the same time. > (Both are guaranteed to be the same length.) > > Previously, we used a pattern of: > > exec_list_iterator 1st_iter = <1st list>.iterator(); > foreach_iter(exec_list_iterator, 2nd_iter, <2nd list>) { > ... > 1st_iter.next(); > } > > This was a bit awkward, since you had to manually iterate through one of > the two lists.
"a bit" lol. > This patch introduces a foreach_list2 macro which safely walks through > two lists at the same time, so you can simply do: > > foreach_list2(1st_node, <1st list>, 2nd_node, <2nd list>) { > ... > } My only suggestion might be to change the name to foreach_two_lists. I think it's more obvious to someone reading the header file looking for utility macros. > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/glsl/ast_function.cpp | 16 ++++---------- > src/glsl/ir.cpp | 12 +++------- > src/glsl/linker.cpp | 9 ++++---- > src/glsl/list.h | 16 ++++++++++++++ > src/glsl/opt_constant_folding.cpp | 9 ++++---- > src/glsl/opt_constant_propagation.cpp | 9 ++++---- > src/glsl/opt_constant_variable.cpp | 9 ++++---- > src/glsl/opt_copy_propagation.cpp | 9 ++++---- > src/glsl/opt_copy_propagation_elements.cpp | 9 ++++---- > src/glsl/opt_function_inlining.cpp | 35 > ++++++++++++------------------ > src/glsl/opt_tree_grafting.cpp | 10 ++++----- > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 22 +++++++------------ > 12 files changed, 73 insertions(+), 92 deletions(-) > > diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp > index e4c0fd1..9a9bb74 100644 > --- a/src/glsl/ast_function.cpp > +++ b/src/glsl/ast_function.cpp > @@ -293,15 +293,10 @@ generate_call(exec_list *instructions, > ir_function_signature *sig, > * call takes place. Since we haven't emitted the call yet, we'll place > * the post-call conversions in a temporary exec_list, and emit them > later. > */ > - exec_list_iterator actual_iter = actual_parameters->iterator(); > - exec_list_iterator formal_iter = sig->parameters.iterator(); > - > - while (actual_iter.has_next()) { > - ir_rvalue *actual = (ir_rvalue *) actual_iter.get(); > - ir_variable *formal = (ir_variable *) formal_iter.get(); > - > - assert(actual != NULL); > - assert(formal != NULL); > + foreach_list2(formal_node, &sig->parameters, > + actual_node, actual_parameters) { > + ir_rvalue *actual = (ir_rvalue *) actual_node; > + ir_variable *formal = (ir_variable *) formal_node; The old code asserts when the lists aren't the same length... or at least when sig->parameters is shorter than actual_parameters. As do the loops in st_glsl_to_tgsi.cpp. I think a debug-build version of foreach_list2 could do the same... I'm just waffling whether there's sufficient value to make it worth doing. Opinions? > if (formal->type->is_numeric() || formal->type->is_boolean()) { > switch (formal->data.mode) { > @@ -323,9 +318,6 @@ generate_call(exec_list *instructions, > ir_function_signature *sig, > break; > } > } > - > - actual_iter.next(); > - formal_iter.next(); > } > > /* If the function call is a constant expression, don't generate any > diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp > index 6ffa987..dcde631 100644 > --- a/src/glsl/ir.cpp > +++ b/src/glsl/ir.cpp > @@ -1649,13 +1649,10 @@ modes_match(unsigned a, unsigned b) > const char * > ir_function_signature::qualifiers_match(exec_list *params) > { > - exec_list_iterator iter_a = parameters.iterator(); > - exec_list_iterator iter_b = params->iterator(); > - > /* check that the qualifiers match. */ > - while (iter_a.has_next()) { > - ir_variable *a = (ir_variable *)iter_a.get(); > - ir_variable *b = (ir_variable *)iter_b.get(); > + foreach_list2(a_node, &this->parameters, b_node, params) { > + ir_variable *a = (ir_variable *) a_node; > + ir_variable *b = (ir_variable *) b_node; > > if (a->data.read_only != b->data.read_only || > !modes_match(a->data.mode, b->data.mode) || > @@ -1666,9 +1663,6 @@ ir_function_signature::qualifiers_match(exec_list > *params) > /* parameter a's qualifiers don't match */ > return a->name; > } > - > - iter_a.next(); > - iter_b.next(); > } > return NULL; > } > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp > index 14e2ff6..7c25031 100644 > --- a/src/glsl/linker.cpp > +++ b/src/glsl/linker.cpp > @@ -109,10 +109,10 @@ public: > > virtual ir_visitor_status visit_enter(ir_call *ir) > { > - exec_list_iterator sig_iter = ir->callee->parameters.iterator(); > - foreach_iter(exec_list_iterator, iter, *ir) { > - ir_rvalue *param_rval = (ir_rvalue *)iter.get(); > - ir_variable *sig_param = (ir_variable *)sig_iter.get(); > + foreach_list2(formal_node, &ir->callee->parameters, > + actual_node, &ir->actual_parameters) { > + ir_rvalue *param_rval = (ir_rvalue *) actual_node; > + ir_variable *sig_param = (ir_variable *) formal_node; > > if (sig_param->data.mode == ir_var_function_out || > sig_param->data.mode == ir_var_function_inout) { > @@ -122,7 +122,6 @@ public: > return visit_stop; > } > } > - sig_iter.next(); > } > > if (ir->return_deref != NULL) { > diff --git a/src/glsl/list.h b/src/glsl/list.h > index 5ac17cb..106772a 100644 > --- a/src/glsl/list.h > +++ b/src/glsl/list.h > @@ -447,6 +447,22 @@ inline void exec_node::insert_before(exec_list *before) > ; (__node)->next != NULL \ > ; (__node) = (__node)->next) > > +/** > + * Iterate through two lists at once. Stops at the end of the shorter list. > + * > + * This is safe against either current node being removed or replaced. > + */ > +#define foreach_list2(__node1, __list1, __node2, __list2) \ > + for (exec_node * __node1 = (__list1)->head, \ > + * __node2 = (__list2)->head, \ > + * __next1 = __node1->next, \ > + * __next2 = __node2->next \ > + ; __next1 != NULL && __next2 != NULL \ > + ; __node1 = __next1, \ > + __node2 = __next2, \ > + __next1 = __next1->next, \ > + __next2 = __next2->next) > + > #define foreach_list_const(__node, __list) \ > for (const exec_node * __node = (__list)->head \ > ; (__node)->next != NULL \ > diff --git a/src/glsl/opt_constant_folding.cpp > b/src/glsl/opt_constant_folding.cpp > index 08a47b9..7a6559e 100644 > --- a/src/glsl/opt_constant_folding.cpp > +++ b/src/glsl/opt_constant_folding.cpp > @@ -122,10 +122,10 @@ ir_visitor_status > ir_constant_folding_visitor::visit_enter(ir_call *ir) > { > /* Attempt to constant fold parameters */ > - exec_list_iterator sig_iter = ir->callee->parameters.iterator(); > - foreach_iter(exec_list_iterator, iter, *ir) { > - ir_rvalue *param_rval = (ir_rvalue *)iter.get(); > - ir_variable *sig_param = (ir_variable *)sig_iter.get(); > + foreach_list2(formal_node, &ir->callee->parameters, > + actual_node, &ir->actual_parameters) { > + ir_rvalue *param_rval = (ir_rvalue *) actual_node; > + ir_variable *sig_param = (ir_variable *) formal_node; > > if (sig_param->data.mode == ir_var_function_in > || sig_param->data.mode == ir_var_const_in) { > @@ -136,7 +136,6 @@ ir_constant_folding_visitor::visit_enter(ir_call *ir) > param_rval->replace_with(new_param); > } > } > - sig_iter.next(); > } > > /* Next, see if the call can be replaced with an assignment of a constant > */ > diff --git a/src/glsl/opt_constant_propagation.cpp > b/src/glsl/opt_constant_propagation.cpp > index a2d1b0f..f87ab26 100644 > --- a/src/glsl/opt_constant_propagation.cpp > +++ b/src/glsl/opt_constant_propagation.cpp > @@ -281,10 +281,10 @@ ir_visitor_status > ir_constant_propagation_visitor::visit_enter(ir_call *ir) > { > /* Do constant propagation on call parameters, but skip any out params */ > - exec_list_iterator sig_param_iter = ir->callee->parameters.iterator(); > - foreach_iter(exec_list_iterator, iter, ir->actual_parameters) { > - ir_variable *sig_param = (ir_variable *)sig_param_iter.get(); > - ir_rvalue *param = (ir_rvalue *)iter.get(); > + foreach_list2(formal_node, &ir->callee->parameters, > + actual_node, &ir->actual_parameters) { > + ir_variable *sig_param = (ir_variable *) formal_node; > + ir_rvalue *param = (ir_rvalue *) actual_node; > if (sig_param->data.mode != ir_var_function_out > && sig_param->data.mode != ir_var_function_inout) { > ir_rvalue *new_param = param; > @@ -294,7 +294,6 @@ ir_constant_propagation_visitor::visit_enter(ir_call *ir) > else > param->accept(this); > } > - sig_param_iter.next(); > } > > /* Since we're unlinked, we don't (necssarily) know the side effects of > diff --git a/src/glsl/opt_constant_variable.cpp > b/src/glsl/opt_constant_variable.cpp > index 22a0fe1..9efa26d 100644 > --- a/src/glsl/opt_constant_variable.cpp > +++ b/src/glsl/opt_constant_variable.cpp > @@ -132,10 +132,10 @@ ir_visitor_status > ir_constant_variable_visitor::visit_enter(ir_call *ir) > { > /* Mark any out parameters as assigned to */ > - exec_list_iterator sig_iter = ir->callee->parameters.iterator(); > - foreach_iter(exec_list_iterator, iter, *ir) { > - ir_rvalue *param_rval = (ir_rvalue *)iter.get(); > - ir_variable *param = (ir_variable *)sig_iter.get(); > + foreach_list2(formal_node, &ir->callee->parameters, > + actual_node, &ir->actual_parameters) { > + ir_rvalue *param_rval = (ir_rvalue *) actual_node; > + ir_variable *param = (ir_variable *) formal_node; > > if (param->data.mode == ir_var_function_out || > param->data.mode == ir_var_function_inout) { > @@ -146,7 +146,6 @@ ir_constant_variable_visitor::visit_enter(ir_call *ir) > entry = get_assignment_entry(var, &this->list); > entry->assignment_count++; > } > - sig_iter.next(); > } > > /* Mark the return storage as having been assigned to */ > diff --git a/src/glsl/opt_copy_propagation.cpp > b/src/glsl/opt_copy_propagation.cpp > index 44c6f2f..5c9e8a1 100644 > --- a/src/glsl/opt_copy_propagation.cpp > +++ b/src/glsl/opt_copy_propagation.cpp > @@ -185,15 +185,14 @@ ir_visitor_status > ir_copy_propagation_visitor::visit_enter(ir_call *ir) > { > /* Do copy propagation on call parameters, but skip any out params */ > - exec_list_iterator sig_param_iter = ir->callee->parameters.iterator(); > - foreach_iter(exec_list_iterator, iter, ir->actual_parameters) { > - ir_variable *sig_param = (ir_variable *)sig_param_iter.get(); > - ir_rvalue *ir = (ir_rvalue *) iter.get(); > + foreach_list2(formal_node, &ir->callee->parameters, > + actual_node, &ir->actual_parameters) { > + ir_variable *sig_param = (ir_variable *) formal_node; > + ir_rvalue *ir = (ir_rvalue *) actual_node; > if (sig_param->data.mode != ir_var_function_out > && sig_param->data.mode != ir_var_function_inout) { > ir->accept(this); > } > - sig_param_iter.next(); > } > > /* Since we're unlinked, we don't (necessarily) know the side effects of > diff --git a/src/glsl/opt_copy_propagation_elements.cpp > b/src/glsl/opt_copy_propagation_elements.cpp > index a64a9ce..555f3a2 100644 > --- a/src/glsl/opt_copy_propagation_elements.cpp > +++ b/src/glsl/opt_copy_propagation_elements.cpp > @@ -293,15 +293,14 @@ ir_visitor_status > ir_copy_propagation_elements_visitor::visit_enter(ir_call *ir) > { > /* Do copy propagation on call parameters, but skip any out params */ > - exec_list_iterator sig_param_iter = ir->callee->parameters.iterator(); > - foreach_iter(exec_list_iterator, iter, ir->actual_parameters) { > - ir_variable *sig_param = (ir_variable *)sig_param_iter.get(); > - ir_rvalue *ir = (ir_rvalue *) iter.get(); > + foreach_list2(formal_node, &ir->callee->parameters, > + actual_node, &ir->actual_parameters) { > + ir_variable *sig_param = (ir_variable *) formal_node; > + ir_rvalue *ir = (ir_rvalue *) actual_node; > if (sig_param->data.mode != ir_var_function_out > && sig_param->data.mode != ir_var_function_inout) { > ir->accept(this); > } > - sig_param_iter.next(); > } > > /* Since we're unlinked, we don't (necessarily) know the side effects of > diff --git a/src/glsl/opt_function_inlining.cpp > b/src/glsl/opt_function_inlining.cpp > index a140ed3..9f165e1 100644 > --- a/src/glsl/opt_function_inlining.cpp > +++ b/src/glsl/opt_function_inlining.cpp > @@ -116,11 +116,10 @@ ir_call::generate_inline(ir_instruction *next_ir) > * and set up the mapping of real function body variables to ours. > */ > i = 0; > - exec_list_iterator sig_param_iter = this->callee->parameters.iterator(); > - exec_list_iterator param_iter = this->actual_parameters.iterator(); > - for (i = 0; i < num_parameters; i++) { > - ir_variable *sig_param = (ir_variable *) sig_param_iter.get(); > - ir_rvalue *param = (ir_rvalue *) param_iter.get(); > + foreach_list2(formal_node, &this->callee->parameters, > + actual_node, &this->actual_parameters) { > + ir_variable *sig_param = (ir_variable *) formal_node; > + ir_rvalue *param = (ir_rvalue *) actual_node; > > /* Generate a new variable for the parameter. */ > if (sig_param->type->contains_opaque()) { > @@ -154,8 +153,7 @@ ir_call::generate_inline(ir_instruction *next_ir) > next_ir->insert_before(assign); > } > > - sig_param_iter.next(); > - param_iter.next(); > + ++i; > } > > exec_list new_instructions; > @@ -172,11 +170,10 @@ ir_call::generate_inline(ir_instruction *next_ir) > /* If any opaque types were passed in, replace any deref of the > * opaque variable with a deref of the argument. > */ > - param_iter = this->actual_parameters.iterator(); > - sig_param_iter = this->callee->parameters.iterator(); > - for (i = 0; i < num_parameters; i++) { > - ir_rvalue *const param = (ir_rvalue *) param_iter.get(); > - ir_variable *sig_param = (ir_variable *) sig_param_iter.get(); > + foreach_list2(formal_node, &this->callee->parameters, > + actual_node, &this->actual_parameters) { > + ir_rvalue *const param = (ir_rvalue *) actual_node; > + ir_variable *sig_param = (ir_variable *) formal_node; > > if (sig_param->type->contains_opaque()) { > ir_dereference *deref = param->as_dereference(); > @@ -184,8 +181,6 @@ ir_call::generate_inline(ir_instruction *next_ir) > assert(deref); > do_variable_replacement(&new_instructions, sig_param, deref); > } > - param_iter.next(); > - sig_param_iter.next(); > } > > /* Now push those new instructions in. */ > @@ -195,11 +190,10 @@ ir_call::generate_inline(ir_instruction *next_ir) > * variables to our own. > */ > i = 0; > - param_iter = this->actual_parameters.iterator(); > - sig_param_iter = this->callee->parameters.iterator(); > - for (i = 0; i < num_parameters; i++) { > - ir_rvalue *const param = (ir_rvalue *) param_iter.get(); > - const ir_variable *const sig_param = (ir_variable *) > sig_param_iter.get(); > + foreach_list2(formal_node, &this->callee->parameters, > + actual_node, &this->actual_parameters) { > + ir_rvalue *const param = (ir_rvalue *) actual_node; > + const ir_variable *const sig_param = (ir_variable *) formal_node; > > /* Move our param variable into the actual param if it's an 'out' > type. */ > if (parameters[i] && (sig_param->data.mode == ir_var_function_out || > @@ -212,8 +206,7 @@ ir_call::generate_inline(ir_instruction *next_ir) > next_ir->insert_before(assign); > } > > - param_iter.next(); > - sig_param_iter.next(); > + ++i; > } > > delete [] parameters; > diff --git a/src/glsl/opt_tree_grafting.cpp b/src/glsl/opt_tree_grafting.cpp > index 6d75a15..884ca4c 100644 > --- a/src/glsl/opt_tree_grafting.cpp > +++ b/src/glsl/opt_tree_grafting.cpp > @@ -204,11 +204,10 @@ > ir_tree_grafting_visitor::visit_enter(ir_function_signature *ir) > ir_visitor_status > ir_tree_grafting_visitor::visit_enter(ir_call *ir) > { > - exec_list_iterator sig_iter = ir->callee->parameters.iterator(); > - /* Reminder: iterating ir_call iterates its parameters. */ > - foreach_iter(exec_list_iterator, iter, *ir) { > - ir_variable *sig_param = (ir_variable *)sig_iter.get(); > - ir_rvalue *ir = (ir_rvalue *)iter.get(); > + foreach_list2(formal_node, &ir->callee->parameters, > + actual_node, &ir->actual_parameters) { > + ir_variable *sig_param = (ir_variable *) formal_node; > + ir_rvalue *ir = (ir_rvalue *) actual_node; > ir_rvalue *new_ir = ir; > > if (sig_param->data.mode != ir_var_function_in > @@ -222,7 +221,6 @@ ir_tree_grafting_visitor::visit_enter(ir_call *ir) > ir->replace_with(new_ir); > return visit_stop; > } > - sig_iter.next(); > } > > if (ir->return_deref && check_graft(ir, ir->return_deref->var) == > visit_stop) > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > index ed3715f..ee60ff4 100644 > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > @@ -2617,10 +2617,10 @@ glsl_to_tgsi_visitor::visit(ir_call *ir) > int i; > > /* Process in parameters. */ > - exec_list_iterator sig_iter = sig->parameters.iterator(); > - foreach_iter(exec_list_iterator, iter, *ir) { > - ir_rvalue *param_rval = (ir_rvalue *)iter.get(); > - ir_variable *param = (ir_variable *)sig_iter.get(); > + foreach_list2(formal_node, &sig->parameters, > + actual_node, &ir->actual_parameters) { > + ir_rvalue *param_rval = (ir_rvalue *) actual_node; > + ir_variable *param = (ir_variable *) formal_node; > > if (param->data.mode == ir_var_function_in || > param->data.mode == ir_var_function_inout) { > @@ -2643,20 +2643,17 @@ glsl_to_tgsi_visitor::visit(ir_call *ir) > r.index++; > } > } > - > - sig_iter.next(); > } > - assert(!sig_iter.has_next()); > > /* Emit call instruction */ > call_inst = emit(ir, TGSI_OPCODE_CAL); > call_inst->function = entry; > > /* Process out parameters. */ > - sig_iter = sig->parameters.iterator(); > - foreach_iter(exec_list_iterator, iter, *ir) { > - ir_rvalue *param_rval = (ir_rvalue *)iter.get(); > - ir_variable *param = (ir_variable *)sig_iter.get(); > + foreach_list2(formal_node, &sig->parameters, > + actual_node, &ir->actual_parameters) { > + ir_rvalue *param_rval = (ir_rvalue *) actual_node; > + ir_variable *param = (ir_variable *) formal_node; > > if (param->data.mode == ir_var_function_out || > param->data.mode == ir_var_function_inout) { > @@ -2679,10 +2676,7 @@ glsl_to_tgsi_visitor::visit(ir_call *ir) > r.index++; > } > } > - > - sig_iter.next(); > } > - assert(!sig_iter.has_next()); > > /* Process return value. */ > this->result = entry->return_reg; > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev