[Mesa-dev] [PATCH 1/2] i965/vec4: Change types as needed to propagate source modifiers using current instruction

[Jason Ekstrand]

On Wed, Sep 16, 2015 at 12:47 PM, Alejandro Piñeiro
<apinheiro at igalia.com> wrote:
> SEL and MOV instructions, as long as they don't have source modifiers, are
> just copying bits around.  So those kind of instruction could be propagated
> even if there are type mismatches. This is needed because NIR generates
> integer SEL and MOV instructions whenever it doesn't know what else to
> generate.
>
> This commit adds support for copy propagation using current instruction
> as reference.
> ---
>
> Equivalent to commit 472ef9 but for the vec4 case.
>
>  .../drivers/dri/i965/brw_vec4_copy_propagation.cpp | 28 ++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> index 5a15eb8..64e2528 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> @@ -249,6 +249,16 @@ try_constant_propagate(const struct brw_device_info *devinfo,
>  }
>
>  static bool
> +can_change_source_types(vec4_instruction *inst)
> +{
> +   return !inst->src[0].abs && !inst->src[0].negate &&
> +      (inst->opcode == BRW_OPCODE_MOV ||
> +       (inst->opcode == BRW_OPCODE_SEL &&
> +        inst->predicate != BRW_PREDICATE_NONE &&
> +        !inst->src[1].abs && !inst->src[1].negate));

You should probably check saturate in here as well.  The only time we
can do saturate propagation is if both the instruction and the MOV are
float type anyway.  I think the FS version has the same theoretical
problem.

With that fixed,

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

> +}
> +
> +static bool
>  try_copy_propagate(const struct brw_device_info *devinfo,
>                     vec4_instruction *inst,
>                     int arg, struct copy_entry *entry)
> @@ -308,7 +318,9 @@ try_copy_propagate(const struct brw_device_info *devinfo,
>          value.swizzle != BRW_SWIZZLE_XYZW) && !inst->can_do_source_mods(devinfo))
>        return false;
>
> -   if (has_source_modifiers && value.type != inst->src[arg].type)
> +   if (has_source_modifiers &&
> +       value.type != inst->src[arg].type &&
> +       !can_change_source_types(inst))
>        return false;
>
>     if (has_source_modifiers &&
> @@ -362,7 +374,19 @@ try_copy_propagate(const struct brw_device_info *devinfo,
>        }
>     }
>
> -   value.type = inst->src[arg].type;
> +   if (has_source_modifiers &&
> +       value.type != inst->src[arg].type) {
> +      /* We are propagating source modifiers from a MOV with a different
> +       * type.  If we got here, then we can just change the source and
> +       * destination types of the instruction and keep going.
> +       */
> +      assert(can_change_source_types(inst));
> +      for (int i = 0; i < 3; i++) {
> +         inst->src[i].type = value.type;
> +      }
> +      inst->dst.type = value.type;
> +   } else
> +      value.type = inst->src[arg].type;
>     inst->src[arg] = value;
>     return true;
>  }
> --
> 2.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev