[Mesa-dev] [PATCH] r600/sb: remove superfluos assert

[Vadim Girlin]

On 09/11/2017 07:09 PM, Emil Velikov wrote:
> On 11 September 2017 at 15:39, Gert Wollny <gw.fossdev at gmail.com> wrote:
>> The assert checks whether pshader->num_arrays != 0, but the code
>> after the assert actually branches based on the same check.
>>
>> Removing this assert fixes:
>>    piglit spec at arb_gpu_shader5@execution at samplemaskin-indirect
> 
> Both assert() and if () have existed since day 1, with below commit.
> Perhaps Vadim has some ideas what happened here?

I guess the assert was added initially just to make sure that I set 
indirect_files and num_arrays fields correctly elsewhere and everything 
related to the indirect arrays works as I expect.

Many features were added since then, so my assumptions from that time 
could be wrong now, I'm just not sure off-hand.

Anyway, if num_arrays is 0 there, I suspect it can be a result of some 
other issue. At the very least it looks like a potential performance 
problem, because in that case we assume all shader registers can be 
accessed with indirect addressing and it can limit the optimizations 
significantly. So it might make sense to figure out why it's zero in the 
first place, in theory it shouldn't happen. Maybe something is wrong 
with the indirect_files bits?

I'm adding Glenn to cc too, AFAIU he has added some related features 
since then, so possibly he knows better.


> 
> Cc: Vadim Girlin <vadimgirlin at gmail.com>
> Fixes: 2cd76917934 ("r600g/sb: initial commit of the optimizing shader backend")
> 
> -Emil
>