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

[Jose Fonseca]

----- Original Message -----
> On Tue, Nov 15, 2011 at 8:58 AM, Jose Fonseca <jfonseca at vmware.com>
> wrote:
> >
> >
> > ----- 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?
> 
> The tricky part is that we use TGSI for more than just GLSL.  The
> pipe
> video stuff and eventually compute, etc.

Sure. But I don't think the TGSI manipulating code in st_mesa_to_tgsi.c / st_glsl_to_tgsi.cpp is used for those paths. Or was that the intention?

Jose