[Mesa-dev] [PATCH 6/8] glsl: Use a new foreach_list2 macro for walking two lists at once.
Ian Romanick
idr at freedesktop.org
Mon Jan 13 09:58:06 PST 2014
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 <kenneth at 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;
>
More information about the mesa-dev
mailing list