[Mesa-dev] [PATCH 04/12] glsl: Lower ir_binop_vector_extract to swizzle

[Ian Romanick]

On 05/05/2013 04:50 PM, Kenneth Graunke wrote:
> On 05/03/2013 04:07 PM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Lower ir_binop_vector_extract with a constant index to a swizzle.  This
>> is exactly like ir_dereference_array of a vector with a constant index.
>>
>> v2: Convert tabs to spaces.  Suggested by Eric.
>>
>> v3: Correctly call convert_vector_extract_to_swizzle in
>> ir_vec_index_to_swizzle_visitor::visit_enter(ir_call *ir).  Suggested by
>> Ken.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Reviewed-by: Eric Anholt <eric at anholt.net>
>> ---
>>   src/glsl/lower_vec_index_to_swizzle.cpp | 47
>> ++++++++++++++++++++++++++++++++-
>>   1 file changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/lower_vec_index_to_swizzle.cpp
>> b/src/glsl/lower_vec_index_to_swizzle.cpp
>> index 264d6dc..9d0d696 100644
>> --- a/src/glsl/lower_vec_index_to_swizzle.cpp
>> +++ b/src/glsl/lower_vec_index_to_swizzle.cpp
>> @@ -47,6 +47,7 @@ public:
>>      }
>>
>>      ir_rvalue *convert_vec_index_to_swizzle(ir_rvalue *val);
>> +   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 *);
>> @@ -93,11 +94,45 @@
>> ir_vec_index_to_swizzle_visitor::convert_vec_index_to_swizzle(ir_rvalue *ir)
>>
>>       * large.  For simplicity sake, just clamp the index to [0, size-1].
>>       */
>>      const int i = MIN2(MAX2(ir_constant->value.i[0], 0),
>> -              ((int) deref->array->type->vector_elements - 1));
>> +                      ((int) deref->array->type->vector_elements - 1));
>
> Looks like a spurious hunk (not a big deal though).
>
>>
>>      return new(ctx) ir_swizzle(deref->array, i, 0, 0, 0, 1);
>>   }
>>
>> +ir_rvalue *
>> +ir_vec_index_to_swizzle_visitor::convert_vector_extract_to_swizzle(ir_rvalue
>> *ir)
>> +{
>> +   ir_expression *const expr = ir->as_expression();
>> +   if (expr == NULL || expr->operation != ir_binop_vector_extract)
>> +      return ir;
>> +
>> +   ir_constant *const idx =
>> expr->operands[1]->constant_expression_value();
>> +   if (idx == NULL)
>> +      return ir;
>> +
>> +   void *ctx = ralloc_parent(ir);
>> +   this->progress = true;
>> +
>> +   /* Page 40 of the GLSL 1.20 spec says:
>> +    *
>> +    *     "When indexing with non-constant expressions, behavior is
>> undefined
>> +    *     if the index is negative, or greater than or equal to the
>> size of
>> +    *     the vector."
>> +    *
>> +    * The quoted spec text mentions non-constant expressions, but
>> this code
>> +    * operates on constants.  These constants are the result of
>> non-constant
>> +    * expressions that have been optimized to constants.  The common
>> case here
>> +    * is a loop counter from an unrolled loop that is used to index a
>> vector.
>> +    *
>> +    * The ir_swizzle constructor gets angry if the index is negative
>> or too
>> +    * large.  For simplicity sake, just clamp the index to [0, size-1].
>> +    */
>> +   const int i = MIN2(MAX2(idx->value.i[0], 0),
>> +                      ((int) expr->operands[0]->type->vector_elements
>> - 1));
>
> Would be easier to read with CLAMP:
>
> const int i = CLAMP(idx->value.i[0], 0,
> int(expr->operands[0]->type->vector_elements - 1));

I always forget about CLAMP.  I'll make that change here and in patch 1.

> Either way, this patch is:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
>> +   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)
>>   {
>> @@ -105,6 +140,7 @@
>> ir_vec_index_to_swizzle_visitor::visit_enter(ir_expression *ir)
>>
>>      for (i = 0; i < ir->get_num_operands(); i++) {
>>         ir->operands[i] = convert_vec_index_to_swizzle(ir->operands[i]);
>> +      ir->operands[i] =
>> convert_vector_extract_to_swizzle(ir->operands[i]);
>>      }
>>
>>      return visit_continue;
>> @@ -127,6 +163,7 @@
>> ir_vec_index_to_swizzle_visitor::visit_enter(ir_assignment *ir)
>>   {
>>      ir->set_lhs(convert_vec_index_to_swizzle(ir->lhs));
>>      ir->rhs = convert_vec_index_to_swizzle(ir->rhs);
>> +   ir->rhs = convert_vector_extract_to_swizzle(ir->rhs);
>>
>>      return visit_continue;
>>   }
>> @@ -140,6 +177,12 @@
>> ir_vec_index_to_swizzle_visitor::visit_enter(ir_call *ir)
>>
>>         if (new_param != param) {
>>        param->replace_with(new_param);
>> +      } else {
>> +         new_param = convert_vector_extract_to_swizzle(param);
>> +
>> +         if (new_param != param) {
>> +            param->replace_with(new_param);
>> +         }
>>         }
>>      }
>>
>> @@ -151,6 +194,7 @@
>> ir_vec_index_to_swizzle_visitor::visit_enter(ir_return *ir)
>>   {
>>      if (ir->value) {
>>         ir->value = convert_vec_index_to_swizzle(ir->value);
>> +      ir->value = convert_vector_extract_to_swizzle(ir->value);
>>      }
>>
>>      return visit_continue;
>> @@ -160,6 +204,7 @@ ir_visitor_status
>>   ir_vec_index_to_swizzle_visitor::visit_enter(ir_if *ir)
>>   {
>>      ir->condition = convert_vec_index_to_swizzle(ir->condition);
>> +   ir->condition = convert_vector_extract_to_swizzle(ir->condition);
>>
>>      return visit_continue;
>>   }
>>
>