2012/10/24 Andreas Boll <[email protected]>: > 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!
Sorry I've pushed this commit accidentally. I've reverted it for now. But I think this patch is ready to land on master. (with the stable note, the reviewed-by and tested-by) Andreas. _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
