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

[Abdiel Janulgue]

On 04.06.2014 15:22, Matt Turner wrote:
> 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 makes it more clear. Thanks for the feedback. I'll have the v2 
coming up soon.
>
> That lets it handle NOT a, -b properly.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>