On Wed, 2016-09-07 at 18:48 -0700, Francisco Jerez wrote:
> The 'scan_inst->dst.offset % REG_SIZE' term in the final
> 'scan_inst->dst.offset' calculation is obviously bogus. The offset
> from the start of the copy destination register 'inst->dst' where the
> destination of the generating instruction 'scan_inst' would be
> written
> to (before compute-to-mrf runs) is just the offset of 'scan_inst-
> >dst'
> relative to the source of the copy instruction (AKA rel_offset in the
> code below).
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 5b7ca98..703340b 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2797,15 +2797,15 @@ fs_visitor::compute_to_mrf()
> inst->src[0], scan_inst->dst, DIV_ROUND_UP(scan_inst-
> >size_written,
> REG_SIZE))
> ;
>
> - const unsigned rel_offset = (reg_offset(scan_inst->dst)
> -
> - reg_offset(inst->src[0])) /
> REG_SIZE;
> + const unsigned rel_offset = reg_offset(scan_inst->dst) -
> + reg_offset(inst->src[0]);
>
> if (inst->dst.nr & BRW_MRF_COMPR4) {
> /* Apply the same address transformation done by the
> hardware
> * for COMPR4 MRF writes.
> */
> - assert(rel_offset < 2);
> - scan_inst->dst.nr = inst->dst.nr + rel_offset * 4;
> + assert(rel_offset < 2 * REG_SIZE);
> + scan_inst->dst.nr = inst->dst.nr + rel_offset /
> REG_SIZE * 4;
>
> /* Clear the COMPR4 bit if the generating instruction
> is not
> * compressed.
> @@ -2817,11 +2817,11 @@ fs_visitor::compute_to_mrf()
> /* Calculate the MRF number the result of this
> instruction is
> * ultimately written to.
> */
> - scan_inst->dst.nr = inst->dst.nr + rel_offset;
> + scan_inst->dst.nr = inst->dst.nr + rel_offset /
> REG_SIZE;
> }
>
> scan_inst->dst.file = MRF;
> - scan_inst->dst.offset = inst->dst.offset + scan_inst-
> >dst.offset % REG_SIZE;
> + scan_inst->dst.offset = inst->dst.offset + rel_offset %
> REG_SIZE;
> scan_inst->saturate |= inst->saturate;
> if (!regs_left)
> break;
Ok, this makes a lot more sense :), maybe squash this and the previous
one together? I found the previous one quite confusing because it seems
to be doing something wrong that is then fixed here.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
