Iago Toral <ito...@igalia.com> writes: > On Mon, 2016-08-08 at 15:26 -0700, Francisco Jerez wrote: >> Iago Toral Quiroga <ito...@igalia.com> writes: >> >> > >> > From the HSW PRM, Command Reference, QtrCtrl: >> > >> > "NibCtrl is only allowed for SIMD4 instructions with a DF >> > (Double Float) >> > source or destination type." >> > >> > v2 (Samuel): Assert that the type is DF. >> > >> > Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com> >> > --- >> > src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 9 +++++++++ >> > 1 file changed, 9 insertions(+) >> > >> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp >> > b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp >> > index c6e040e..dc4f6db 100644 >> > --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp >> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp >> > @@ -1494,6 +1494,7 @@ generate_code(struct brw_codegen *p, >> > brw_set_default_saturate(p, inst->saturate); >> > brw_set_default_mask_control(p, inst->force_writemask_all); >> > brw_set_default_acc_write_control(p, inst- >> > >writes_accumulator); >> > + brw_set_default_group(p, 0); >> > >> > assert(inst->base_mrf + inst->mlen <= BRW_MAX_MRF(devinfo- >> > >gen)); >> > assert(inst->mlen <= BRW_MAX_MSG_LENGTH); >> > @@ -1529,6 +1530,14 @@ generate_code(struct brw_codegen *p, >> > } else { >> > assert(inst->exec_size == 8 || inst->exec_size == 4); >> > brw_set_default_exec_size(p, cvt(inst->exec_size) - 1); >> > + if (inst->exec_size == 4 && !inst->force_writemask_all) { >> > + assert(inst->dst.type == BRW_REGISTER_TYPE_DF || >> > + inst->src[0].type == BRW_REGISTER_TYPE_DF || >> > + inst->src[1].type == BRW_REGISTER_TYPE_DF || >> > + inst->src[2].type == BRW_REGISTER_TYPE_DF); >> > + assert(inst->group == 0 || inst->group == 4); >> > + brw_inst_set_group(p->devinfo, p->current, inst- >> > >group); >> I think this should be unconditional (e.g. if somebody tries to set >> group == 4 on an instruction with exec_size == 8 it would be nice to >> get >> an assertion failure instead of the group value being silently >> ignored). How about you replace the 'brw_set_default_group(p, 0)' >> line >> above with: >> >> > >> > >> > assert(inst->group % inst->exec_size == 0); >> > assert(inst->group % 8 == 0 || >> > inst->dst.type == BRW_REGISTER_TYPE_DF || >> > inst->src[0].type == BRW_REGISTER_TYPE_DF || >> > inst->src[1].type == BRW_REGISTER_TYPE_DF || >> > inst->src[2].type == BRW_REGISTER_TYPE_DF); >> > brw_set_default_group(p, inst->group); >> and then drop this hunk? > > Yes, that looks like better. Thanks! > > I suppose that setting the group won't have any effect if > force_writemask_all is set as too so there is no need to void setting > it in that case, right? > Yeah, it shouldn't make much of a difference, I wouldn't bother not to set it.
>> > >> > } >> > >> > switch (inst->opcode) {
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev