[Mesa-dev] [PATCH] Plumb invariant output attrib thru TGSI

Jakob Bornecrantz jakob at collabora.com
Mon Jun 18 11:06:42 UTC 2018


TGSI already has a invariant field on declarations, we are running
into a bug with virgl because st_glsl_to_tgsi completely drops the
invariant flag on the floor when it comes to declarations (tho precise
is added to the ops). But virgl can't express precise ops (only
invariant and precise declarations) only declarations.

Going to do some testing on this patch soon.

Cheers, Jakob.

On Tue, Apr 10, 2018 at 7:02 PM Marek Olšák <maraeo at gmail.com> wrote:
>
> This doesn't change TGSI. It only changes utilities around it.
>
> Marek
>
> On Mon, Apr 9, 2018 at 6:02 PM, Joe M. Kniss <djmk at chromium.org> wrote:
>>
>> Add support for glsl 'invariant' modifier for output data declarations.
>> Gallium drivers that use TGSI serialization currently loose invariant
>> modifiers in glsl shaders.
>>
>> Tested: chromiumos on qemu with virglrenderer.
>> Signed-off-by: Joe M. Kniss <djmk at google.com>
>> ---
>>  src/gallium/auxiliary/tgsi/tgsi_strings.c  |  2 ++
>>  src/gallium/auxiliary/tgsi/tgsi_strings.h  |  2 ++
>>  src/gallium/auxiliary/tgsi/tgsi_text.c     | 18 +++++++++++----
>>  src/gallium/auxiliary/tgsi/tgsi_ureg.c     | 27 ++++++++++++++--------
>>  src/gallium/auxiliary/tgsi/tgsi_ureg.h     |  4 +++-
>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  8 +++++--
>>  6 files changed, 45 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_strings.c b/src/gallium/auxiliary/tgsi/tgsi_strings.c
>> index 4f28b49ce8..434871273f 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_strings.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_strings.c
>> @@ -185,6 +185,8 @@ const char *tgsi_interpolate_locations[TGSI_INTERPOLATE_LOC_COUNT] =
>>     "SAMPLE",
>>  };
>>
>> +const char *tgsi_invariant_name = "INVARIANT";
>> +
>>  const char *tgsi_primitive_names[PIPE_PRIM_MAX] =
>>  {
>>     "POINTS",
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_strings.h b/src/gallium/auxiliary/tgsi/tgsi_strings.h
>> index bb2d3458dd..20e3f7127f 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_strings.h
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_strings.h
>> @@ -52,6 +52,8 @@ extern const char *tgsi_interpolate_names[TGSI_INTERPOLATE_COUNT];
>>
>>  extern const char *tgsi_interpolate_locations[TGSI_INTERPOLATE_LOC_COUNT];
>>
>> +extern const char *tgsi_invariant_name;
>> +
>>  extern const char *tgsi_primitive_names[PIPE_PRIM_MAX];
>>
>>  extern const char *tgsi_fs_coord_origin_names[2];
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> index 02241a66bf..815b1ee65d 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> @@ -1586,10 +1586,6 @@ static boolean parse_declaration( struct translate_ctx *ctx )
>>              break;
>>           }
>>        }
>> -      if (i == TGSI_INTERPOLATE_COUNT) {
>> -         report_error( ctx, "Expected semantic or interpolate attribute" );
>> -         return FALSE;
>> -      }
>>     }
>>
>>     cur = ctx->cur;
>> @@ -1609,6 +1605,20 @@ static boolean parse_declaration( struct translate_ctx *ctx )
>>        }
>>     }
>>
>> +   cur = ctx->cur;
>> +   eat_opt_white( &cur );
>> +   if (*cur == ',' && !is_vs_input) {
>> +      cur++;
>> +      eat_opt_white( &cur );
>> +      if (str_match_nocase_whole( &cur, tgsi_invariant_name )) {
>> +         decl.Declaration.Invariant = 1;
>> +         ctx->cur = cur;
>> +      } else {
>> +         report_error( ctx, "Expected semantic, interpolate attribute, or invariant ");
>> +         return FALSE;
>> +      }
>> +   }
>> +
>>     advance = tgsi_build_full_declaration(
>>        &decl,
>>        ctx->tokens_cur,
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> index 393e015001..f54e2229a7 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> @@ -140,6 +140,7 @@ struct ureg_program
>>        unsigned first;
>>        unsigned last;
>>        unsigned array_id;
>> +      unsigned invariant;
>>     } output[UREG_MAX_OUTPUT];
>>     unsigned nr_outputs, nr_output_regs;
>>
>> @@ -427,7 +428,8 @@ ureg_DECL_output_layout(struct ureg_program *ureg,
>>                          unsigned index,
>>                          unsigned usage_mask,
>>                          unsigned array_id,
>> -                        unsigned array_size)
>> +                        unsigned array_size,
>> +                        unsigned invariant)
>>  {
>>     unsigned i;
>>
>> @@ -455,6 +457,7 @@ ureg_DECL_output_layout(struct ureg_program *ureg,
>>        ureg->output[i].first = index;
>>        ureg->output[i].last = index + array_size - 1;
>>        ureg->output[i].array_id = array_id;
>> +      ureg->output[i].invariant = invariant;
>>        ureg->nr_output_regs = MAX2(ureg->nr_output_regs, index + array_size);
>>        ureg->nr_outputs++;
>>     }
>> @@ -480,7 +483,7 @@ ureg_DECL_output_masked(struct ureg_program *ureg,
>>                          unsigned array_size)
>>  {
>>     return ureg_DECL_output_layout(ureg, name, index, 0,
>> -                                  ureg->nr_output_regs, usage_mask, array_id, array_size);
>> +                                  ureg->nr_output_regs, usage_mask, array_id, array_size, 0);
>>  }
>>
>>
>> @@ -1512,7 +1515,8 @@ emit_decl_semantic(struct ureg_program *ureg,
>>                     unsigned semantic_index,
>>                     unsigned streams,
>>                     unsigned usage_mask,
>> -                   unsigned array_id)
>> +                   unsigned array_id,
>> +                   unsigned invariant)
>>  {
>>     union tgsi_any_token *out = get_tokens(ureg, DOMAIN_DECL, array_id ? 4 : 3);
>>
>> @@ -1523,7 +1527,8 @@ emit_decl_semantic(struct ureg_program *ureg,
>>     out[0].decl.UsageMask = usage_mask;
>>     out[0].decl.Semantic = 1;
>>     out[0].decl.Array = array_id != 0;
>> -
>> +   out[0].decl.Invariant = invariant;
>> +
>>     out[1].value = 0;
>>     out[1].decl_range.First = first;
>>     out[1].decl_range.Last = last;
>> @@ -1870,7 +1875,8 @@ static void emit_decls( struct ureg_program *ureg )
>>                                 ureg->input[i].semantic_index,
>>                                 0,
>>                                 TGSI_WRITEMASK_XYZW,
>> -                               ureg->input[i].array_id);
>> +                               ureg->input[i].array_id,
>> +                               0);
>>           }
>>        }
>>        else {
>> @@ -1883,7 +1889,7 @@ static void emit_decls( struct ureg_program *ureg )
>>                                    ureg->input[i].semantic_index +
>>                                    (j - ureg->input[i].first),
>>                                    0,
>> -                                  TGSI_WRITEMASK_XYZW, 0);
>> +                                  TGSI_WRITEMASK_XYZW, 0, 0);
>>              }
>>           }
>>        }
>> @@ -1897,7 +1903,7 @@ static void emit_decls( struct ureg_program *ureg )
>>                           ureg->system_value[i].semantic_name,
>>                           ureg->system_value[i].semantic_index,
>>                           0,
>> -                         TGSI_WRITEMASK_XYZW, 0);
>> +                         TGSI_WRITEMASK_XYZW, 0, 0);
>>     }
>>
>>     if (ureg->supports_any_inout_decl_range) {
>> @@ -1910,7 +1916,8 @@ static void emit_decls( struct ureg_program *ureg )
>>                              ureg->output[i].semantic_index,
>>                              ureg->output[i].streams,
>>                              ureg->output[i].usage_mask,
>> -                            ureg->output[i].array_id);
>> +                            ureg->output[i].array_id,
>> +                            ureg->output[i].invariant);
>>        }
>>     }
>>     else {
>> @@ -1923,7 +1930,9 @@ static void emit_decls( struct ureg_program *ureg )
>>                                 ureg->output[i].semantic_index +
>>                                 (j - ureg->output[i].first),
>>                                 ureg->output[i].streams,
>> -                               ureg->output[i].usage_mask, 0);
>> +                               ureg->output[i].usage_mask,
>> +                               0,
>> +                               ureg->output[i].invariant);
>>           }
>>        }
>>     }
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> index 7eef94a65e..46523c7aa6 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> @@ -79,6 +79,7 @@ struct ureg_dst
>>     unsigned DimIndirect     : 1;  /* BOOL */
>>     unsigned Dimension       : 1;  /* BOOL */
>>     unsigned Saturate        : 1;  /* BOOL */
>> +   unsigned Invariant       : 1;  /* BOOL */
>>     int      Index           : 16; /* SINT */
>>     int      IndirectIndex   : 16; /* SINT */
>>     unsigned IndirectFile    : 4;  /* TGSI_FILE_ */
>> @@ -250,7 +251,8 @@ ureg_DECL_output_layout(struct ureg_program *,
>>                          unsigned index,
>>                          unsigned usage_mask,
>>                          unsigned array_id,
>> -                        unsigned array_size);
>> +                        unsigned array_size,
>> +                        unsigned invariant);
>>
>>  struct ureg_dst
>>  ureg_DECL_output_masked(struct ureg_program *,
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 5f7a0dcd3e..2eb5243f7c 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -123,6 +123,7 @@ struct inout_decl {
>>     enum glsl_interp_mode interp;
>>     enum glsl_base_type base_type;
>>     ubyte usage_mask; /* GLSL-style usage-mask,  i.e. single bit per double */
>> +   unsigned invariant;
>>  };
>>
>>  static struct inout_decl *
>> @@ -2508,6 +2509,8 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir)
>>           unsigned num_components;
>>           num_outputs++;
>>
>> +         decl->invariant = var->data.invariant;
>> +
>>           if (type_without_array->is_64bit())
>>              component = component / 2;
>>           if (type_without_array->vector_elements)
>> @@ -6437,14 +6440,15 @@ st_translate_program(
>>                       (enum tgsi_semantic) outputSemanticName[slot],
>>                       outputSemanticIndex[slot],
>>                       decl->gs_out_streams,
>> -                     slot, tgsi_usage_mask, decl->array_id, decl->size);
>> -
>> +                     slot, tgsi_usage_mask, decl->array_id, decl->size, decl->invariant);
>> +         dst.Invariant = decl->invariant;
>>           for (unsigned j = 0; j < decl->size; ++j) {
>>              if (t->outputs[slot + j].File != TGSI_FILE_OUTPUT) {
>>                 /* The ArrayID is set up in dst_register */
>>                 t->outputs[slot + j] = dst;
>>                 t->outputs[slot + j].ArrayID = 0;
>>                 t->outputs[slot + j].Index += j;
>> +               t->outputs[slot + j].Invariant = decl->invariant;
>>              }
>>           }
>>        }
>> --
>> 2.17.0.484.g0c8726318c-goog
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list