[Mesa-dev] [PATCH 13/13] glsl: Compare vector indices in blocks

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/21/2011 05:28 PM, Eric Anholt wrote:
> On Thu, 21 Jul 2011 12:16:58 -0700, "Ian Romanick" <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Just like the non-constant array index lowering pass, compare all N
>> indices at once.  For accesses to a vec4, this saves 3 comparison
>> instructions on a vector architecture.
>> ---
>>  src/glsl/lower_vec_index_to_cond_assign.cpp |   57 ++++++++++++++++----------
>>  1 files changed, 35 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/glsl/lower_vec_index_to_cond_assign.cpp b/src/glsl/lower_vec_index_to_cond_assign.cpp
>> index 15992e2..26c5452 100644
>> --- a/src/glsl/lower_vec_index_to_cond_assign.cpp
>> +++ b/src/glsl/lower_vec_index_to_cond_assign.cpp
>> @@ -93,32 +93,40 @@ ir_vec_index_to_cond_assign_visitor::convert_vec_index_to_cond_assign(ir_rvalue
>>     base_ir->insert_before(index);
>>     deref = new(base_ir) ir_dereference_variable(index);
>>     assign = new(base_ir) ir_assignment(deref, orig_deref->array_index, NULL);
>> -   base_ir->insert_before(assign);
>> +   list.push_tail(assign);
>>  
>>     /* Temporary where we store whichever value we swizzle out. */
>>     var = new(base_ir) ir_variable(ir->type, "vec_index_tmp_v",
>>  				  ir_var_temporary);
>> -   base_ir->insert_before(var);
>> +   list.push_tail(var);
>> +
>> +   /* 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++) {
>> -      deref = new(base_ir) ir_dereference_variable(index);
>> -      condition = new(base_ir) ir_expression(ir_binop_equal,
>> -					     glsl_type::bool_type,
>> -					     deref,
>> -					     new(base_ir) ir_constant(i));
>> +      ir_rvalue *condition_swizzle =
>> +	 new(ir) ir_swizzle(cond_deref->clone(ir, NULL), i, 0, 0, 0, 1);
> 
> If you've got the mem_ctx = ralloc_parent(base_ir) around already, it seems
> like the right thing to use here (and in a few other places in this code)

That was something odd about the original code.  My addition of
'new(ir)' was a copy-and-paste mistake.  I had intended it to be
'new(base_ir)' to follow the original code.  But yeah.  Either mem_ctx
should be removed, it should be used everywhere, or there should be a
comment explaining why the code is weird.

For the purpose of this commit, I'll just change the 'new(ir)' to
'new(base_ir)'.

> Other than that, switching the base_ir->insert_before(whatever) to
> list.push_tail() in various places (but not everywhere) seems strange.

The only instance of that is to put the whole set of new instructions in
the IR stream.  I'll add a comment.

The reason it uses list.push_tail is because compare_index_block wants a
list as a parameter, so there's a list for at least some of the added
instructions.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk4ppPgACgkQX1gOwKyEAw9UowCfRE0Rnr0Uh3/odeT7YQVEHm3y
RPgAnRh7kynlETxG7BY4zaLiFPTXZfiU
=iaXq
-----END PGP SIGNATURE-----