[Mesa-dev] [PATCH 10/25] i965/fs: Generalize regions_overlap() from copy propagation to handle non-VGRF files.

[Jason Ekstrand]

On Fri, May 27, 2016 at 7:05 PM, Francisco Jerez <currojerez at riseup.net>
wrote:

> This will be useful in several places.  The only externally visible
> difference (other than non-VGRF files being supported now) is that the
> region sizes are now passed in byte units instead of in GRF units
> because the loss of precision would have become a problem in the SIMD
> lowering pass.
> ---
>  .../drivers/dri/i965/brw_fs_copy_propagation.cpp   | 16 +++------
>  src/mesa/drivers/dri/i965/brw_ir_fs.h              | 38
> ++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> index 38103af..d88d62b 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -757,14 +757,6 @@ can_propagate_from(fs_inst *inst)
>             !inst->is_partial_write());
>  }
>
> -inline bool
> -regions_overlap(const fs_reg &r, unsigned n, const fs_reg &s, unsigned m)
> -{
> -   return r.file == s.file && r.nr == s.nr &&
> -      !(r.reg_offset + n <= s.reg_offset ||
> -        s.reg_offset + m <= r.reg_offset);
> -}
> -
>  /* Walks a basic block and does copy propagation on it using the acp
>   * list.
>   */
> @@ -791,8 +783,8 @@ fs_visitor::opt_copy_propagate_local(void
> *copy_prop_ctx, bblock_t *block,
>        /* kill the destination from the ACP */
>        if (inst->dst.file == VGRF) {
>           foreach_in_list_safe(acp_entry, entry, &acp[inst->dst.nr %
> ACP_HASH_SIZE]) {
> -            if (regions_overlap(entry->dst, entry->regs_written,
> -                                inst->dst, inst->regs_written))
> +            if (regions_overlap(entry->dst, entry->regs_written *
> REG_SIZE,
> +                                inst->dst, inst->regs_written * REG_SIZE))
>                 entry->remove();
>           }
>
> @@ -804,8 +796,8 @@ fs_visitor::opt_copy_propagate_local(void
> *copy_prop_ctx, bblock_t *block,
>                 /* Make sure we kill the entry if this instruction
> overwrites
>                  * _any_ of the registers that it reads
>                  */
> -               if (regions_overlap(entry->src, entry->regs_read,
> -                                   inst->dst, inst->regs_written))
> +               if (regions_overlap(entry->src, entry->regs_read *
> REG_SIZE,
> +                                   inst->dst, inst->regs_written *
> REG_SIZE))
>                    entry->remove();
>              }
>          }
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> index cc1561d..73c6327 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> @@ -172,6 +172,44 @@ component(fs_reg reg, unsigned idx)
>  }
>
>  /**
> + * Return an integer identifying the discrete address space a register is
> + * contained in.  A register is by definition fully contained in the
> single
> + * reg_space it belongs to, so two registers with different reg_space ids
> are
> + * guaranteed not to overlap.  Most register files are a single reg_space
> of
> + * its own, only the VGRF file is composed of multiple discrete address
> + * spaces, one for each VGRF allocation.
> + */
> +static inline uint32_t
> +reg_space(const fs_reg &r)
> +{
> +   return r.file << 16 | (r.file == VGRF ? r.nr : 0);
> +}
>

I think I like this concept.  It would lead to a bit more compact numbering
if you did "return r.file == VGRF ? r.nr + 8 : r.file".  Obviously, you
would probably want to replace "8" with a BRW_NUM_REG_FILES constant.

Given that we really only care about reg_space(r) == reg_space(s) right
now, this is fine.

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> +
> +/**
> + * Return the base offset in bytes of a register relative to the start of
> its
> + * reg_space().
> + */
> +static inline unsigned
> +reg_offset(const fs_reg &r)
> +{
> +   return ((r.file == VGRF || r.file == IMM ? 0 : r.nr) + r.reg_offset) *
> +          (r.file == UNIFORM ? 4 : REG_SIZE) + r.subreg_offset;
> +}
> +
> +/**
> + * Return whether the register region starting at \p r and spanning \p dr
> + * bytes could potentially overlap the register region starting at \p s
> and
> + * spanning \p ds bytes.
> + */
> +static inline bool
> +regions_overlap(const fs_reg &r, unsigned dr, const fs_reg &s, unsigned
> ds)
> +{
> +   return reg_space(r) == reg_space(s) &&
> +          !(reg_offset(r) + dr <= reg_offset(s) ||
> +            reg_offset(s) + ds <= reg_offset(r));
> +}
> +
> +/**
>   * Return whether the given register region is n-periodic, i.e. whether
> the
>   * original region remains invariant after shifting it by \p n scalar
>   * channels.
> --
> 2.7.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160528/615712e9/attachment.html>