[Mesa-dev] [PATCH 3/5] i965/fs: Add a helper function for checking for partial register updates.

[Matt Turner]

On Wed, Apr 10, 2013 at 11:54 AM, Eric Anholt <eric at anholt.net> wrote:
> These checks were all over, and every time I wrote one I had to try to
> decide again what the cases were for partial updates.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp               |   35 +++++++++++---------
>  src/mesa/drivers/dri/i965/brw_fs.h                 |    1 +
>  .../drivers/dri/i965/brw_fs_copy_propagation.cpp   |    4 +--
>  src/mesa/drivers/dri/i965/brw_fs_cse.cpp           |    3 +-
>  .../drivers/dri/i965/brw_fs_live_variables.cpp     |    4 +--
>  5 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index c12ba45..331e1ea 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -711,6 +711,22 @@ fs_visitor::pop_force_sechalf()
>  }
>
>  /**
> + * Returns true if the instruction has a flag that means it won't
> + * update an entire destination register.
> + *
> + * For example, dead code elimination and live variable analysis want to know
> + * when a write to a variable screens off any preceding values that were in
> + * it.
> + */
> +bool
> +fs_inst::is_partial_write()
> +{
> +   return (this->predicate ||
> +           this->force_uncompressed ||
> +           this->force_sechalf);
> +}
> +
> +/**
>   * Returns how many MRFs an FS opcode will write over.
>   *
>   * Note that this is not the 0 or 1 implied writes in an actual gen
> @@ -2065,22 +2081,14 @@ fs_visitor::compute_to_mrf()
>              * into a compute-to-MRF.
>              */
>
> -           /* If it's predicated, it (probably) didn't populate all
> -            * the channels.  We might be able to rewrite everything
> +           /* If this one instruction didn't populate all the
> +            * channels, bail.  We might be able to rewrite everything
>              * that writes that reg, but it would require smarter
>              * tracking to delay the rewriting until complete success.
>              */
> -           if (scan_inst->predicate)
> +           if (scan_inst->is_partial_write())
>                break;
>
> -           /* If it's half of register setup and not the same half as
> -            * our MOV we're trying to remove, bail for now.
> -            */
> -           if (scan_inst->force_uncompressed != inst->force_uncompressed ||
> -               scan_inst->force_sechalf != inst->force_sechalf) {
> -              break;
> -           }
> -
>              /* Things returning more than one register would need us to
>               * understand coalescing out more than one MOV at a time.
>               */
> @@ -2662,10 +2670,7 @@ fs_visitor::get_instruction_generating_reg(fs_inst *start,
>                                            fs_reg reg)
>  {
>     if (end == start ||
> -       end->predicate ||
> -       end->force_uncompressed ||
> -       end->force_sechalf ||
> -       reg.reladdr ||

Is the removal of reg.reladdr intentional?

> +       end->is_partial_write() ||
>         !reg.equals(end->dst)) {
>        return NULL;
>     } else {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 60e3e0a..f0901e7 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -179,6 +179,7 @@ public:
>     bool is_math();
>     bool is_control_flow();
>     bool is_send_from_grf();
> +   bool is_partial_write();
>
>     fs_reg dst;
>     fs_reg src[3];
> 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 36df759..234f8bd 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -414,9 +414,7 @@ fs_visitor::opt_copy_propagate_local(void *mem_ctx, bblock_t *block,
>             inst->src[0].file == IMM) &&
>           inst->src[0].type == inst->dst.type &&
>           !inst->saturate &&
> -         !inst->predicate &&
> -         !inst->force_uncompressed &&
> -         !inst->force_sechalf) {
> +         !inst->is_partial_write()) {
>          acp_entry *entry = ralloc(mem_ctx, acp_entry);
>          entry->dst = inst->dst;
>          entry->src = inst->src[0];
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> index 2a8fd0b..b5c2200 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> @@ -97,8 +97,7 @@ fs_visitor::opt_cse_local(bblock_t *block, exec_list *aeb)
>         inst = (fs_inst *) inst->next) {
>
>        /* Skip some cases. */
> -      if (is_expression(inst) && !inst->predicate &&
> -          !inst->force_uncompressed && !inst->force_sechalf &&
> +      if (is_expression(inst) && !inst->is_partial_write() &&
>            !inst->conditional_mod)
>        {
>          bool found = false;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> index ca60aa2..fdcfac6 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> @@ -78,9 +78,7 @@ fs_live_variables::setup_def_use()
>           */
>          if (inst->dst.file == GRF &&
>              inst->regs_written == v->virtual_grf_sizes[inst->dst.reg] &&
> -            !inst->predicate &&
> -            !inst->force_uncompressed &&
> -            !inst->force_sechalf) {
> +            !inst->is_partial_write()) {
>             int reg = inst->dst.reg;
>              if (!BITSET_TEST(bd[b].use, reg))
>                 BITSET_SET(bd[b].def, reg);
> --
> 1.7.10.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev