[Mesa-dev] [PATCH 3/6] i965/fs: skip copy-propate for negated logical instructions and 'NOT' src registers

[Matt Turner]

On Tue, Jun 3, 2014 at 3:59 PM, Abdiel Janulgue
<abdiel.janulgue at linux.intel.com> wrote:
> The negation source modifier on src registers has changed meaning in Broadwell when
> used with logical operations.
>
> Make sure copy propagation occurs only for original statements that does not have
> negated source registers and destination instruction is not a logical op. In addition,
> since we have added 'NOT' as a potentially propagate-able instruction, don't
> propagate it either when the destination instruction is not a logical op.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> 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 09d5949..26eda92 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -273,6 +273,15 @@ fs_copy_prop_dataflow::dump_block_data() const
>     }
>  }
>
> +static bool
> +is_logic_op(enum opcode opcode)
> +{
> +   return (opcode == BRW_OPCODE_AND ||
> +           opcode == BRW_OPCODE_OR  ||
> +           opcode == BRW_OPCODE_XOR ||
> +           opcode == BRW_OPCODE_NOT);
> +}
> +
>  bool
>  fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
>  {
> @@ -331,6 +340,11 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
>     if (has_source_modifiers && entry->dst.type != inst->src[arg].type)
>        return false;
>
> +   if (brw->gen >=8 &&
> +       ((entry->src.negate && is_logic_op(inst->opcode)) ||
> +        (entry->opcode == BRW_OPCODE_NOT && !is_logic_op(inst->opcode))))
> +      return false;

The static function and some bit of this logic need to go into the
previous patch. (And some bits, like adding opcode to the entry needs
to go in this patch)

This conditional doesn't allow copy propagation from

not a, -b
and c, d, a

since it return false if gen >= 8 && is_logic_op(inst) &&
entry->src.negate (which in the case of a NOT, means ~, not negation).
It's a trivial case, but fixing it should entail splitting the
conditional and making it clearer.

Maybe we want something like:

if (brw->gen >= 8) {
   if (entry->opcode == BRW_OPCODE_NOT) {
      if (!is_logic_op(inst->opcode)) {
         return false;
      }
   } else if (entry->src.negate) {
      if (is_logic_op(inst->opcode) {
         return false;
      }
   }
}

That lets it handle NOT a, -b properly.