[Mesa-dev] [PATCH (resend)] r600/sb: Don't require array declarations for TGSI_FILE_SYSTEM_VALUE

[Ilia Mirkin]

On Fri, Feb 2, 2018 at 7:55 AM, Gert Wollny <gw.fossdev at gmail.com> wrote:
> Am Freitag, den 02.02.2018, 06:56 -0500 schrieb Ilia Mirkin:
>> On Fri, Feb 2, 2018 at 4:07 AM, Gert Wollny <gw.fossdev at gmail.com>
>> wrote:
>> > Am Freitag, den 02.02.2018, 09:04 +0100 schrieb Roland Scheidegger:
>> > >
>> > >
>> > > Yes, the _GL spec_ says it is an array.
>> > > But in gallium it can't be. Therefore I think it's incorrect if
>> > > we
>> > > end up with array accesses there (albeit I was too lazy to
>> > > actually
>> > > look at the tgsi, but I'm pretty sure it isn't declard as an
>> > > array).
>> >
>> > the TGSI for the relevant shader in the piglit looks like this:
>> >
>> > 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
>>
>> OK, this is a big problem. I'm guessing the GLSL code was something
>> like
>>
>> gl_SampleMaskIn[uniform]
>>
> This is how the piglit is written, and the standard definess
> gl_SampleMaskIn is defined as an array, so this makes sense.
>
>> What this got translated into was an indirect access into the *global
>> implicit array of system values*. We don't want that. glsl_to_tgsi
>> should just be dropping the indirect access entirely.
> Just found this comment in mesa/st/st_glsl_to.tgsi.cpp:6495
>
>  "TODO: If we ever support more than 32 samples, this will have
>    to become an array."
>
> which would imply to me that in this stage the indirect access might at
> one point become relevant.

Pretty sure I wrote that :) (commit c5d822dad90)

I added it as part of ARB_sample_shading which just has a
gl_SampleMask output in FS. I'm sure I didn't think about indirect
accesses at the time. (Not even sure they'd be legal on the output...
I guess probably would.)

To support indirect accesses, one would have to declare an arrayid,
probably -- either way, it'd be a whole thing to support it. For now,
it should be disallowed and that indirect arg should get dropped.

  -ilia