On Mon, Feb 9, 2015 at 12:26 PM, 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)
> v3: The 3-source instruction bit 105 is part of SourceIndex on CHV.
> Add assertion that reserved bit 7 is not set. (Matt)
> Document overlap with UIP and 64-bit immediate fields.
> ---
> src/mesa/drivers/dri/i965/brw_eu_compact.c | 48
> ++++++++++++++++++++++++++++++
> 1 file changed, 48 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..cb93636 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> @@ -843,10 +843,54 @@ 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)
> + * - Imm64[27:31] (bits 91-95 on Gen7, bit 95 on Gen8)
> + * - UIP[31] (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);
> + 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));
> +}
> +
> +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, 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);
Thanks for adding the assertion for bit 7. Do you think it's valuable
to check it here as well? I wouldn't think it was.
What I'm suggesting is that we only do assertions about reserved bits.
If they're set, we'll trigger the assertion only in debug builds, but
I think that's sufficient. I think we should only check useful bits
here (i.e., non-reserverd bits in the uncompacted instruction that
don't exist in the compacted instruction).
I think effectively that means removing the checks for bit 7, all of
the bits in the 3-src BDW case (always return false), and 90 on gen <
7.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
