[Mesa-dev] [PATCH] gallium/ttn: mark location specially in nir for color0-writes-all

[Rob Clark]

On Sat, Jun 27, 2015 at 7:47 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> On Sat, Jun 27, 2015 at 2:54 PM, Rob Clark <robdclark at gmail.com> wrote:
>> Connor, btw, how does glsl_to_nir handle this?
>
> The variable for gl_FragColor a special location (FRAG_RESULT_COLOR),
> whereas normal outputs (i.e. gl_FragData[] or out variables) start at
> FRAG_RESULT_DATA0.
>
>>
>> I guess at some point we want to try to align ttn w/ gtn (ie. cleanup
>> the whole nir with a tgsi accent thing)..
>>
>> not that this would be the worst of the ttn vs gtn diff's in the
>> current state, so it is fine if there isn't a better way to do it
>> right now.. but I would like to start moving towards using nir
>> #define's for semantic names/indexes, etc, rather than tgsi or glsl
>> specific ones..  in the end the driver shouldn't have to care if the
>> nir came from tgsi/glsl/spirv/etc
>
> Indeed, it would be nice to have NIR do it's own, well-documented
> thing that's consistent between all the frontends... I've been looking
> at SPIR-V inputs/outputs, and it has its own way of doing things
> that's a little more flexible than GLSL in some ways (e.g. right now
> you can have arbitrary structs of structs and one of the members can
> be marked as flat or noperspective), although it's not really clear
> how much of this is intentional and how much is because it wasn't
> specified or accidentally allowed. The plan has been to map that onto
> how the existing Mesa infrastructure does things, although the current
> underspecification of SPIR-V combined with the underspecification of
> how it all works in Mesa has been a bit frustrating. I'm not really
> familiar with how it works in Gallium at all, though, so I'm not sure
> I'd be the one for the job, or if there even is a person really
> qualified for it.

the one good thing about tgsi is it is pretty well specified, and in a
way that is independent of what sits upstream of tgsi ;-)

I'd be a fan of specifying things in NIR (w/ it's own
enums/#defines/etc) and mapping glsl/spirv/etc onto that.. current
re-use of glsl specific stuff in NIR is a bit of a pain point for
gallium drivers (since gallium drivers can be used for things other
than gl..)

BR,
-R

>>
>> BR,
>> -R
>>
>>
>> On Sat, Jun 27, 2015 at 5:38 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> We need to distinguish a shader that has separate writes to each MRT
>>> from one which is supposed to write the data from MRT 0 to all the MRTs.
>>> In TGSI this is done with a property. NIR doesn't have that, so encode
>>> it as a funny location and decode on the other end.
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>
>>> This fixes bin/fbo-drawbuffers-none gl_FragColor when I additionally
>>> initialize the "default" colors to a register other than r0.x -- as is
>>> this happens to work by luck.
>>>
>>> Also fix up vc4 to ignore this for now.
>>>
>>>  src/gallium/auxiliary/nir/tgsi_to_nir.c              | 7 ++++++-
>>>  src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c | 4 ++++
>>>  src/gallium/drivers/vc4/vc4_program.c                | 6 ++++++
>>>  3 files changed, 16 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/gallium/auxiliary/nir/tgsi_to_nir.c b/src/gallium/auxiliary/nir/tgsi_to_nir.c
>>> index bf7eb2f..4130697 100644
>>> --- a/src/gallium/auxiliary/nir/tgsi_to_nir.c
>>> +++ b/src/gallium/auxiliary/nir/tgsi_to_nir.c
>>> @@ -253,7 +253,12 @@ ttn_emit_declaration(struct ttn_compile *c)
>>>              var->name = ralloc_asprintf(var, "out_%d", idx);
>>>
>>>              var->data.location = decl->Semantic.Name;
>>> -            var->data.index = decl->Semantic.Index;
>>> +            if (decl->Semantic.Name == TGSI_SEMANTIC_COLOR &&
>>> +                decl->Semantic.Index == 0 &&
>>> +                c->scan->properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS])
>>> +               var->data.index = -1;
>>> +            else
>>> +               var->data.index = decl->Semantic.Index;
>>>
>>>              if (is_array) {
>>>                 unsigned j;
>>> diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
>>> index 3b36114..fa13c40 100644
>>> --- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
>>> +++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
>>> @@ -2110,6 +2110,10 @@ setup_output(struct ir3_compile *ctx, nir_variable *out)
>>>                         so->writes_pos = true;
>>>                         break;
>>>                 case TGSI_SEMANTIC_COLOR:
>>> +                       if (semantic_index == -1) {
>>> +                               semantic_index = 0;
>>> +                               so->color0_mrt = 1;
>>> +                       }
>>>                         break;
>>>                 default:
>>>                         compile_error(ctx, "unknown FS semantic name: %s\n",
>>> diff --git a/src/gallium/drivers/vc4/vc4_program.c b/src/gallium/drivers/vc4/vc4_program.c
>>> index 2061631..728ecc6 100644
>>> --- a/src/gallium/drivers/vc4/vc4_program.c
>>> +++ b/src/gallium/drivers/vc4/vc4_program.c
>>> @@ -1783,6 +1783,12 @@ ntq_setup_outputs(struct vc4_compile *c)
>>>
>>>                  assert(array_len == 1);
>>>
>>> +                /* NIR hack to pass through
>>> +                 * TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS */
>>> +                if (semantic_name == TGSI_SEMANTIC_COLOR &&
>>> +                    semantic_index == -1)
>>> +                        semantic_index = 0;
>>> +
>>>                  for (int i = 0; i < 4; i++) {
>>>                          add_output(c,
>>>                                     loc + i,
>>> --
>>> 2.3.6
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev