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

[Jose Fonseca]

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

Yes. I seldom touched the st_foo_to_tgsi.c but always thought to my self that IR manipulation should had happened before the TGSI translation.  However it was not clear if things were done as they are because of unfamiliarity with glsl internals, or some limitation in glsl passes.

For example, reusing temporaries registers (so that the total number of temporaries is minimized), could this be done in a glsl pass?

Jose