On Mon, Jun 18, 2018 at 1:06 PM Elie Tournier <tournier.e...@gmail.com> wrote:
>
> On Mon, Jun 18, 2018 at 12:06:42PM +0100, Jakob Bornecrantz wrote:
> > 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.
> Supposing that it doesn't brake any gallium driver.
> Maybe Marek can confirm. I only test on qemu with virgl.
>
> Reviewed-by: Elie Tournier <elie.tourn...@collabora.com>
Did a run with this patch using r600 on the host and virgl in the
guest, ran the Android GLES[2-3] CTS in the host and the guest, saw no
regressions so patch is
Tested-by: Jakob Bornecrantz <ja...@collabora.com>
As Elie said we might want to get somebody else to look at this as
this will no plumb down invariant where it in the past did not.
Cheers, Jakob.
>
> >
> > On Tue, Apr 10, 2018 at 7:02 PM Marek Olšák <mar...@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 <d...@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 <d...@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@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > >
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
