[Mesa-dev] [PATCH] glsl_to_tgsi: set correct register type for array and structure elements

[Bryan Cain]

On 10/23/2012 04:50 PM, Brian Paul wrote:
> On 10/23/2012 10:58 AM, Bryan Cain wrote:
>> This fixes an issue where glsl_to_tgsi_visior::get_opcode() would
>> emit the
>> wrong opcode because the register type was GLSL_TYPE_ARRAY/STRUCT
>> instead of
>> GLSL_TYPE_FLOAT/INT/UINT/BOOL, so the function would use the float
>> opcodes for
>> operations on integer or boolean values dereferenced from an array or
>> structure.  Assertions have been added to get_opcode() to prevent
>> this bug
>> from reappearing in the future.
>> ---
>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   21
>> +++++++++++++++++++--
>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 9146f24..cefc568 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -633,6 +633,11 @@ glsl_to_tgsi_visitor::get_opcode(ir_instruction
>> *ir, unsigned op,
>>   {
>>      int type = GLSL_TYPE_FLOAT;
>>
>> +   assert(src0.type != GLSL_TYPE_ARRAY);
>> +   assert(src0.type != GLSL_TYPE_STRUCT);
>> +   assert(src1.type != GLSL_TYPE_ARRAY);
>> +   assert(src1.type != GLSL_TYPE_STRUCT);
>> +
>>      if (src0.type == GLSL_TYPE_FLOAT || src1.type == GLSL_TYPE_FLOAT)
>>         type = GLSL_TYPE_FLOAT;
>>      else if (native_integers)
>> @@ -1074,8 +1079,12 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
>>                  assert(index == storage->index + (int)i);
>>               }
>>            } else {
>> -            st_src_reg src(PROGRAM_STATE_VAR, index,
>> -                  native_integers ? ir->type->base_type :
>> GLSL_TYPE_FLOAT);
>> +             /* We use GLSL_TYPE_FLOAT here regardless of the actual
>> type of
>> +              * the data being moved since MOV does not care about
>> the type of
>> +              * data it is moving, and we don't want to declare
>> registers with
>> +              * array or struct types.
>> +              */
>> +            st_src_reg src(PROGRAM_STATE_VAR, index, GLSL_TYPE_FLOAT);
>>               src.swizzle = slots[i].swizzle;
>>               emit(ir, TGSI_OPCODE_MOV, dst, src);
>>               /* even a float takes up a whole vec4 reg in a
>> struct/array. */
>> @@ -2042,6 +2051,9 @@
>> glsl_to_tgsi_visitor::visit(ir_dereference_array *ir)
>>      else
>>         src.swizzle = SWIZZLE_NOOP;
>>
>> +   /* Change the register type to the element type of the array. */
>> +   src.type = ir->type->base_type;
>> +
>>      this->result = src;
>>   }
>>
>> @@ -2067,6 +2079,7 @@
>> glsl_to_tgsi_visitor::visit(ir_dereference_record *ir)
>>         this->result.swizzle = SWIZZLE_NOOP;
>>
>>      this->result.index += offset;
>> +   this->result.type = ir->type->base_type;
>>   }
>>
>>   /**
>> @@ -2286,6 +2299,10 @@ glsl_to_tgsi_visitor::visit(ir_assignment *ir)
>>         inst->dead_mask = inst->dst.writemask;
>>      } else {
>>         for (i = 0; i<  type_size(ir->lhs->type); i++) {
>> +         if (ir->rhs->type->is_array())
>> +             r.type = ir->rhs->type->element_type()->base_type;
>> +         else if (ir->rhs->type->is_record())
>> +             r.type =
>> ir->rhs->type->fields.structure[i].type->base_type;
>>            emit(ir, TGSI_OPCODE_MOV, l, r);
>>            l.index++;
>>            r.index++;
>
> Thanks.  That seems to fix the test program that Jose posted to the
> piglit list (vs-all-equal-bool-array).
>
> Did you do a full piglit regression test?
>
> If so:
>
> Reviewed-by: Brian Paul <brianp at vmware.com>
>
> But we should probably note this as a candidate for the stable branches.
>
> -Brian

I did a piglit regression test with "-t shaders", which I think should
cover everything.  And yes, this should be a candidate for the stable
branches.  I just forgot to mention that in the commit message.

Bryan