On Mon, Feb 9, 2015 at 6:08 AM, Francisco Jerez <curroje...@riseup.net> wrote: > Some instruction bits don't have a mapping defined to any compacted > instruction field. If they're ever set and we end up compacting the > instruction they will be forced to zero. Avoid using compaction in such > cases. > > v2: Align multiple lines of an expression to the same column. Change > conditional compaction of 3-source instructions to an > assertion. (Matt) > --- > src/mesa/drivers/dri/i965/brw_eu_compact.c | 46 > ++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c > b/src/mesa/drivers/dri/i965/brw_eu_compact.c > index 8e33bcb..f80bcc1 100644 > --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c > +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c > @@ -843,10 +843,53 @@ set_3src_source_index(struct brw_context *brw, > brw_compact_inst *dst, brw_inst * > } > > static bool > +has_unmapped_bits(struct brw_context *brw, brw_inst *src) > +{ > + /* Check for instruction bits that don't map to any of the fields of the > + * compacted instruction. The instruction cannot be compacted if any of > + * them are set. They overlap with: > + * - NibCtrl (bit 47 on Gen7, bit 11 on Gen8) > + * - Dst.AddrImm[9] (bit 47 on Gen8) > + * - Src0.AddrImm[9] (bit 95 on Gen8) > + */ > + if (brw->gen >= 8) > + return brw_inst_bits(src, 95, 95) || > + brw_inst_bits(src, 47, 47) || > + brw_inst_bits(src, 11, 11) || > + brw_inst_bits(src, 7, 7);
11 (NibCtrl) is the only one that isn't reserved. I'd rather assert that reserved bits are not set. > + else > + return brw_inst_bits(src, 95, 91) || > + brw_inst_bits(src, 47, 47) || > + brw_inst_bits(src, 7, 7) || > + (brw->gen < 7 && brw_inst_bits(src, 90, 90)); Same thing, 47 (NibCtrl) is the only non-reserved bit. I'd rather assert about the others. > +} > + > +static bool > +has_3src_unmapped_bits(struct brw_context *brw, brw_inst *src) > +{ > + /* Check for three-source instruction bits that don't map to any of the > + * fields of the compacted instruction. All of them seem to be reserved > + * bits currently. > + */ > + assert(brw->gen >= 8); > + if (brw->gen >= 9 || brw->is_cherryview) > + return brw_inst_bits(src, 127, 127) || > + brw_inst_bits(src, 105, 105) || 105 is Src1's SubRegNum[word] on CHV and is included in SourceIndex. > + brw_inst_bits(src, 7, 7); > + else > + return brw_inst_bits(src, 127, 126) || > + brw_inst_bits(src, 105, 105) || > + brw_inst_bits(src, 84, 84) || > + brw_inst_bits(src, 36, 35) || > + brw_inst_bits(src, 7, 7); Since bit 7 is reserved in all cases, we might just assert(brw_inst_bits(src, 7, 7) == 0) in brw_try_compact_instruction(). I don't have a preference. So, I think it'd be nice to differentiate between bits that aren't included in compact instructions and reserved bits. (Sorry, I could have given you this feedback in the first time around) _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev