[Mesa-dev] [PATCH 2/2] freedreno: ir3: fix wrong return if reg is an array

[zzoon at igalia.com]

On 10/24/18 11:05 PM, Rob Clark wrote:
> On Tue, Oct 23, 2018 at 9:57 PM Hyunjun Ko <zzoon at igalia.com> wrote:
>> Since ir3_register struct has union, it could return true even
>> if it's an array register accidentally when checking whether it
>> is address/predicate register.
>>
>> Fixes: dEQP-GLES31.functional.ssbo.layout.random.arrays_of_arrays.6
>> ---
>>   src/gallium/drivers/freedreno/ir3/ir3.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/freedreno/ir3/ir3.h b/src/gallium/drivers/freedreno/ir3/ir3.h
>> index 3055c10f1d..db94603558 100644
>> --- a/src/gallium/drivers/freedreno/ir3/ir3.h
>> +++ b/src/gallium/drivers/freedreno/ir3/ir3.h
>> @@ -739,7 +739,7 @@ static inline bool writes_addr(struct ir3_instruction *instr)
>>   {
>>          if (instr->regs_count > 0) {
>>                  struct ir3_register *dst = instr->regs[0];
>> -               return reg_num(dst) == REG_A0;
>> +               return (reg_num(dst) == REG_A0) && !(dst->flags & IR3_REG_ARRAY);
> hmm, good catch.. although I wonder if writes_gpr() in ir3_ra.c has
> the same issue.  Or anywhere else?  (Otoh, I guess checking for CONST
> and IMMED in writes_gpr() is a bit silly)
>
>  From a quick look at the IR3_REG_* flags, I think IR3_REG_ARRAY is the
> only problematic case.. but this is a bit more fragile than it should
> be.  Maybe we should just move 'int num' out of the union?  Maybe that
> was too much premature optimization?
>
> BR,
> -R

Didn't figure out writes_gpr() might have same issue.
I think taking num out of union should avoid other similar issues if we 
don't think
that would increase memory footprint that much.

Working on this.


>
>>          }
>>          return false;
>>   }
>> @@ -748,7 +748,7 @@ static inline bool writes_pred(struct ir3_instruction *instr)
>>   {
>>          if (instr->regs_count > 0) {
>>                  struct ir3_register *dst = instr->regs[0];
>> -               return reg_num(dst) == REG_P0;
>> +               return (reg_num(dst) == REG_P0) && !(dst->flags & IR3_REG_ARRAY);
>>          }
>>          return false;
>>   }
>> --
>> 2.17.1
>>