[Mesa-dev] [PATCH 05/10] i965/vec4: Mark TCS_OPCODE_SRC0_010_IS_ZERO as writing the flag.

[Matt Turner]

On Mon, Mar 14, 2016 at 5:10 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>> Missing this causes an assertion failure in the scheduler with the next
>> patch.
>> ---
>>  src/mesa/drivers/dri/i965/brw_ir_vec4.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> index 660beca..7cedf8e 100644
>> --- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> @@ -201,7 +201,8 @@ public:
>>     {
>>        return (conditional_mod && (opcode != BRW_OPCODE_SEL &&
>>                                    opcode != BRW_OPCODE_IF &&
>> -                                  opcode != BRW_OPCODE_WHILE));
>> +                                  opcode != BRW_OPCODE_WHILE)) ||
>> +             opcode == TCS_OPCODE_SRC0_010_IS_ZERO;
>
> Meh...  Any reason this weird instruction doesn't have the
> conditional_mod set on creation?  Having the generator set it implicitly
> makes the instruction *less* useful and is the only reason we need to
> introduce these hacks.

None that I know of. I'd be happy with that fix as a replacement for this patch.

> AFAICT the TCS_OPCODE_SRC0_010_IS_ZERO opcode is actually redundant and
> equivalent to:
>
>  broadcast.nz.f0 null.xyzw, src.xxxx, 0UD

I think there's a difference -- that broadcast instruction treats the
two vec4 halves independently, reading a component from each of them.
The SRC0_010_IS_ZERO reads a single component of the whole vec8 (I
don't see calls that would set it, but that region implies it's an
align1 instruction). Does that seem right?

> Do you feel like cleaning this up?  Otherwise:
>
> Acked-by: Francisco Jerez <currojerez at riseup.net>