Jason Ekstrand <ja...@jlekstrand.net> writes: > Prior to gen8, the flag reg and subreg numbers are in different > locations on 3src instructions than on smaller instructions. In order > for brw_set_default_flag_reg to work properly, we need to copy the value > out of the 2src location and write it into the 3src location as part of > brw_alu3. > > Cc: mesa-sta...@lists.freedesktop.org > --- > src/intel/compiler/brw_eu_emit.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/intel/compiler/brw_eu_emit.c > b/src/intel/compiler/brw_eu_emit.c > index 4f51d51..fc39d94 100644 > --- a/src/intel/compiler/brw_eu_emit.c > +++ b/src/intel/compiler/brw_eu_emit.c > @@ -824,6 +824,18 @@ brw_alu3(struct brw_codegen *p, unsigned opcode, struct > brw_reg dest, > dest.type == BRW_REGISTER_TYPE_DF || > dest.type == BRW_REGISTER_TYPE_D || > dest.type == BRW_REGISTER_TYPE_UD); > + > + /* Flag registers are in a different spot on 3src instructions so we > + * need to move the value if we want brw_set_default_flag_reg to work > + * properly. > + */ > + unsigned flag_reg_nr = > + devinfo->gen >= 7 ? brw_inst_flag_reg_nr(devinfo, inst) : 0; > + unsigned flag_subreg_nr = brw_inst_flag_subreg_nr(devinfo, inst); > + if (devinfo->gen >= 7) > + brw_inst_set_3src_a16_flag_reg_nr(devinfo, inst, flag_reg_nr); > + brw_inst_set_3src_a16_flag_subreg_nr(devinfo, inst, flag_subreg_nr); > +
This seems really gross... brw_next_insn() with a 3-source opcode gives you a corrupted 3-source instruction initialized as if it were a 2-source one. This fixes the 3-source flag register field hoping that all other fields of p->current will magically match the 3-source instruction layout, and hoping that the stray bits of the 2-source flag register field will be overwritten by something else in this function. Proper fix would be to get rid of the p->current thing altogether IMO and use a mechanism to track instruction defaults based on their semantics rather than on the binary encoding of an instruction which has unknown encoding... That would be rather invasive though, a compromise might be to fix it in brw_next_insn() by starting with a clean instruction and initializing it from scratch with the 3src helpers... > if (devinfo->gen == 6) { > brw_inst_set_3src_a16_dst_reg_file(devinfo, inst, > dest.file == > BRW_MESSAGE_REGISTER_FILE); > -- > 2.5.0.400.gff86faf > > _______________________________________________ > mesa-stable mailing list > mesa-sta...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-stable
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev