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

[Jose Fonseca]

----- Original Message -----
> 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,

The way to fix this is to allocate a consecutive range of temps at start, when there are indirect writes to output registers and at least one read.

> 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?

The drawback of doing this is that TGSI will look even more different between drivers. This means that when somebody makes a change to the state tracker, and tested with one driver, it may break other drivers. This also means that comparing a driver to llvmpipe/softpipe will be less meaningfull, as different paths will be taken in the state tracker.

I'm not sure if for this particular issue a new cap is worth or not. Just pointing out that there are downsides of breaking orthogonality between state tracker and driver. It should not be done lightly.


Jose