[Mesa-dev] [PATCH] i965/vec4: do not trim dead channels on gen6 for math

[Tapani Pälli]

On 04/02/2014 09:45 AM, Matt Turner wrote:
> On Tue, Apr 1, 2014 at 11:18 PM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> On GEN6 math operations require destination writemask to be WRITEMASK_XYZW.
> That's not really it. Gen6 doesn't support the math in align16 mode,
> which is the mode required for writemasking and swizzling.

Ah yep, I forgot about the align16, Topi also pointed this out for me.

>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76883
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> index 32a3892..3ed1bad 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> @@ -322,8 +322,10 @@ src_reg::equals(src_reg *r)
>>  }
>>
>>  static bool
>> -try_eliminate_instruction(vec4_instruction *inst, int new_writemask)
>> +try_eliminate_instruction(vec4_instruction *inst, int new_writemask,
>> +                          const struct brw_context *brw)
>>  {
>> +
> Extra newline.
>
>>     if (new_writemask == 0) {
>>        /* Don't dead code eliminate instructions that write to the
>>         * accumulator as a side-effect. Instead just set the destination
>> @@ -344,6 +346,7 @@ try_eliminate_instruction(vec4_instruction *inst, int new_writemask)
>>        }
>>        return true;
>>     } else if (inst->dst.writemask != new_writemask) {
>> +
> Extra newline.
>
>>        switch (inst->opcode) {
>>        case SHADER_OPCODE_TXF_CMS:
>>        case SHADER_OPCODE_GEN4_SCRATCH_READ:
>> @@ -351,6 +354,10 @@ try_eliminate_instruction(vec4_instruction *inst, int new_writemask)
>>        case VS_OPCODE_PULL_CONSTANT_LOAD_GEN7:
>>           break;
>>        default:
>> +         /* On GEN6, vec4 backend for math requires a full mask. */
> Same comment as the about the commit message.
>
>> +         if (brw->gen == 6 && inst->is_math())
> Just fold this into the existing conditional below?

OK, I think it deserves some commenting though and that is why I
separated it.

I'll cleanup the patch and send v2.

>> +            return false;
>> +
>>           if (!inst->is_tex()) {
>>              inst->dst.writemask = new_writemask;
>>              return true;
>> @@ -407,7 +414,8 @@ vec4_visitor::dead_code_eliminate()
>>              }
>>           }
>>
>> -         progress = try_eliminate_instruction(inst, write_mask) || progress;
>> +         progress = try_eliminate_instruction(inst, write_mask, brw) ||
>> +                    progress;
>>        }
>>
>>        if (seen_control_flow || inst->predicate || inst->prev == NULL)
>> @@ -456,7 +464,7 @@ vec4_visitor::dead_code_eliminate()
>>           if (inst->dst.reg == scan_inst->dst.reg) {
>>              int new_writemask = scan_inst->dst.writemask & ~dead_channels;
>>
>> -            progress = try_eliminate_instruction(scan_inst, new_writemask) ||
>> +            progress = try_eliminate_instruction(scan_inst, new_writemask, brw) ||
>>                         progress;
>>           }
>>
>> --
>> 1.8.3.1