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

[Glenn Kennard]

On Tue, 12 Sep 2017 19:25:18 +0200, Vadim Girlin <vadimgirlin at gmail.com> wrote:

> On 09/12/2017 12:49 PM, Gert Wollny wrote:
>> Am Dienstag, den 12.09.2017, 09:56 +0300 schrieb Vadim Girlin:
>>> On 09/11/2017 07:09 PM, Emil Velikov wrote:
>>
>>> 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?files
>>
>> The shader that's failing is this (i.e. no arrays, and indirect access
>> only to SV).
>
> Is the tested feature really supported by r600g? AFAICS the indirect
> index value is unused in the shader code.
>
> Anyway, at first glance it looks like we don't need indirect addressing
> for GPRs in this case, so the outer "if" around that assert probably
> should handle this case too and skip the assert. I'm not 100% sure though.
>
>>
>> FRAG
>> DCL SV[0], SAMPLEMASK
>> DCL OUT[0], COLOR
>> DCL CONST[0][0]
>> DCL TEMP[0..1], LOCAL
>> DCL ADDR[0]
>> IMM[0] FLT32 {    1.0000,     0.0000,     0.0000,     0.0000}
>> IMM[1] INT32 {1, 0, 0, 0}
>>    0: MOV TEMP[0], IMM[0].xyyx
>>    1: UARL ADDR[0].x, CONST[0][0].xxxx
>>    2: USEQ TEMP[1].x, SV[ADDR[0].x].xxxx, IMM[1].xxxx
>>    3: UIF TEMP[1].xxxx
>>    4:   MOV TEMP[0].xy, IMM[0].yxyy
>>    5: ENDIF
>>    6: MOV OUT[0], TEMP[0]
>>    7: END
>>
>> ===== SHADER #12 ======================================
>> PS/BARTS/EVERGREEN =====
>> ===== 36 dw ===== 8 gprs ===== 1 stack
>> =========================================
>> 0000  40000005 a4180000 ALU_PUSH_BEFORE 7 @10 KC0[CB0:0-15]
>> 0010  000000f9 00400c90     1     x: MOV                R2.x,  1.0
>> 0012  000004f8 20400c90           y: MOV                R2.y,  0
>> 0014  000004f8 40400c90           z: MOV                R2.z,  0
>> 0016  000000f9 60400c90           w: MOV                R2.w,  1.0
>> 0018  80000080 00800c90           t: MOV                R4.x,  KC0[0].x
>> 0020  801f4800 00601d10     2     x: SETE_INT           R3.x,  R0.z, 1
>> 0022  801f00fe 00e0229c     3 MP  x: PRED_SETNE_INT     R7.x,  PV.x, 0
>> 0002  00000003 82800001 JUMP @6 POP:1
>> 0004  0000000c a8040000 ALU_POP_AFTER 2 @24
>> 0024  000004f8 00400c90     4     x: MOV                R2.x,  0
>> 0026  800000f9 20400c90           y: MOV                R2.y,  1.0
>> 0006  0000000e a00c0000 ALU 4 @28
>> 0028  00000002 00200c90     5     x: MOV                R1.x,  R2.x
>> 0030  00000402 20200c90           y: MOV                R1.y,  R2.y
>> 0032  00000802 40200c90           z: MOV                R1.z,  R2.z
>> 0034  80000c02 60200c90           w: MOV                R1.w,  R2.w
>> 0008  c0008000 95200688 EXPORT_DONE        PIXEL 0     R1.xyzw  EOP
>> ===== SHADER_END
>>
>
>

Hi Gert,

Vadim is correct, the fix is to extend the check in the if case above to also exclude TGSI_FILE_SYSTEM_VALUE, and keep the assert in place. ie:

  if (pshader->indirect_files & ~((1 << TGSI_FILE_CONSTANT) | (1 << TGSI_FILE_SAMPLER) | (1 << TGSI_FILE_SYSTEM_VALUE))) {


Although gl_SampleMaskIn is declared as an array in GLSL, its effectively a 32 bit mask on all hardware supported by mesa so the array indexing is simply ignored. Thanks for looking in to this!


/Glenn