2012/10/24 Marek Olšák <[email protected]>: > 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
Tested-by: Andreas Boll <[email protected]> I found no regression in piglit (quick-driver) on r600g (rv770), llvmpipe and softpipe. This patch fixes the following piglit tests: {fs,vs}-bool-array on r600g and softpipe {fs,vs}-int-array on r600g and softpipe vs-all-equal-bool-array on llvmpipe Those tests pass for me on mesa 8.0.4. So this patch actually fixes regressions introduced somewhere on the road. Nice work Bryan! _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
