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

[Brian Paul]

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