On Tue, Oct 23, 2012 at 11:52 PM, Bryan Cain <[email protected]> wrote: > 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 <[email protected]> >> >> 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.
"-t shaders" actually covers a very small subset of all GLSL tests. Most of them are in the spec group. Marek _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
