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

[Connor Abbott]

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.

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