[Mesa-dev] Allowing the reading of outputs for some drivers

[Ian Romanick]

On 11/13/2011 03:06 AM, Vadim Girlin wrote:
> Hi,
>
> I found some problem with glsl_to_tgsi: remove_output_reads function.
> It's replacing outputs with temps, producing incorrect results with
> relative addressing. You can see it e.g. with
> "vs-varying-array-mat2-col-rd.shader_test". Here is a dump:
>
>> VERT
>> DCL IN[0]
>> DCL OUT[0], POSITION
>> DCL OUT[1], GENERIC[12]
>> DCL OUT[2], GENERIC[13]
>> DCL OUT[3], GENERIC[14]
>> DCL OUT[4], GENERIC[15]
>> DCL OUT[5], GENERIC[16]
>> DCL OUT[6], GENERIC[17]
>> DCL OUT[7], GENERIC[18]
>> DCL CONST[0..5]
>> DCL TEMP[0..1]
>> DCL ADDR[0]
>> IMM FLT32 {    1.0000,     2.0000,     3.0000,     4.0000}
>> IMM FLT32 {    5.0000,     6.0000,     7.0000,     8.0000}
>> IMM FLT32 {    9.0000,    10.0000,    11.0000,    12.0000}
>> IMM FLT32 {    0.0000,     1.0000,     0.0000,     0.0000}
>>    0: MUL TEMP[0], CONST[2], IN[0].xxxx
>>    1: MAD TEMP[1], CONST[3], IN[0].yyyy, TEMP[0]
>>    2: MAD TEMP[1], CONST[4], IN[0].zzzz, TEMP[1]
>>    3: MAD OUT[0], CONST[5], IN[0].wwww, TEMP[1]
>>    4: MOV OUT[2], IMM[0].xyyy
>>    5: MOV OUT[3], IMM[0].zwww
>>    6: MOV TEMP[0], IMM[1].xyyy
>
> OUT[2-7] is a "varying mat2x2[3] m;", OUT[4] is replaced with the temp
> in the instruction 6.
>
>>    7: MOV OUT[5], IMM[1].zwww
>>    8: MOV OUT[6], IMM[2].xyyy
>>    9: MOV OUT[7], IMM[2].zwww
>>   10: ARL ADDR[0].x, CONST[1].xxxx
>>   11: SNE TEMP[1], TEMP[ADDR[0].x].xyyy, CONST[0].xyyy
>
> Instruction 11 contains the read with the relative addressing using
> this temp, which is incorrect.
>
> I'm not sure how to fix it, but AFAICS at least for r600g this step
> could be skipped completely - r600 can read outputs without any
> problem, they are located in the general-purpose registers. Removing
> calls to remove_output_reads and assert(src.File != TGSI_FILE_OUTPUT)
> in the ureg_emit_src produces correct result and test passes on
> evergreen (total number of fixed tests is about 60).
>
> Probably it makes sense to make this step optional and ask the driver
> whether to use it, if I'm not missing something?

I'd say the right answer is to make a lowering pass that operates on the 
GLSL IR, *NOT* on TGSI.  The st_glsl_to_tgsi does a lot of operations on 
TGSI IR that would be much easier and be much better suited to the 
higher-level IR.
>
> Vadim
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>