Hi Tim, I had to make a couple of small tweaks to get this to apply against 19.0 (namely that the glsl_type_is_struct -> glsl_type_is_struct_or_ifc doesn't exist in 19.0), could you take a look at the patch in the staging/19.0 branch and let me know if it looks okay?
Thanks, Dylan Quoting Timothy Arceri (2019-04-24 18:17:42) > We were only setting the used mask for the first component of a > varying. Since the linking opts split vectors into scalars this > has mostly worked ok. > > However this causes an issue where for example if we split a > struct on one side of the interface but not the other, then we > can possibly end up removing the first components on the side > that was split and then incorrectly remove the whole struct > on the other side of the varying. > > With this change we simply mark all 4 components for each slot > used by a struct. We could possibly make this more fine gained > but that would require a more complex change. > > This fixes a bug in Strange Brigade on RADV when tessellation > is enabled, all credit goes to Samuel Pitoiset for tracking down > the cause of the bug. > > Fixes: f1eb5e639997 ("nir: add component level support to > remove_unused_io_vars()") > --- > src/compiler/nir/nir_linking_helpers.c | 51 +++++++++++++++++--------- > 1 file changed, 33 insertions(+), 18 deletions(-) > > diff --git a/src/compiler/nir/nir_linking_helpers.c > b/src/compiler/nir/nir_linking_helpers.c > index f4494c78f98..ef0f94baf47 100644 > --- a/src/compiler/nir/nir_linking_helpers.c > +++ b/src/compiler/nir/nir_linking_helpers.c > @@ -59,6 +59,15 @@ get_variable_io_mask(nir_variable *var, gl_shader_stage > stage) > return ((1ull << slots) - 1) << location; > } > > +static uint8_t > +get_num_components(nir_variable *var) > +{ > + if (glsl_type_is_struct_or_ifc(glsl_without_array(var->type))) > + return 4; > + > + return glsl_get_vector_elements(glsl_without_array(var->type)); > +} > + > static void > tcs_add_output_reads(nir_shader *shader, uint64_t *read, uint64_t > *patches_read) > { > @@ -80,12 +89,14 @@ tcs_add_output_reads(nir_shader *shader, uint64_t *read, > uint64_t *patches_read) > continue; > > nir_variable *var = nir_deref_instr_get_variable(deref); > - if (var->data.patch) { > - patches_read[var->data.location_frac] |= > - get_variable_io_mask(var, shader->info.stage); > - } else { > - read[var->data.location_frac] |= > - get_variable_io_mask(var, shader->info.stage); > + for (unsigned i = 0; i < get_num_components(var); i++) { > + if (var->data.patch) { > + patches_read[var->data.location_frac + i] |= > + get_variable_io_mask(var, shader->info.stage); > + } else { > + read[var->data.location_frac + i] |= > + get_variable_io_mask(var, shader->info.stage); > + } > } > } > } > @@ -161,22 +172,26 @@ nir_remove_unused_varyings(nir_shader *producer, > nir_shader *consumer) > uint64_t patches_read[4] = { 0 }, patches_written[4] = { 0 }; > > nir_foreach_variable(var, &producer->outputs) { > - if (var->data.patch) { > - patches_written[var->data.location_frac] |= > - get_variable_io_mask(var, producer->info.stage); > - } else { > - written[var->data.location_frac] |= > - get_variable_io_mask(var, producer->info.stage); > + for (unsigned i = 0; i < get_num_components(var); i++) { > + if (var->data.patch) { > + patches_written[var->data.location_frac + i] |= > + get_variable_io_mask(var, producer->info.stage); > + } else { > + written[var->data.location_frac + i] |= > + get_variable_io_mask(var, producer->info.stage); > + } > } > } > > nir_foreach_variable(var, &consumer->inputs) { > - if (var->data.patch) { > - patches_read[var->data.location_frac] |= > - get_variable_io_mask(var, consumer->info.stage); > - } else { > - read[var->data.location_frac] |= > - get_variable_io_mask(var, consumer->info.stage); > + for (unsigned i = 0; i < get_num_components(var); i++) { > + if (var->data.patch) { > + patches_read[var->data.location_frac + i] |= > + get_variable_io_mask(var, consumer->info.stage); > + } else { > + read[var->data.location_frac + i] |= > + get_variable_io_mask(var, consumer->info.stage); > + } > } > } > > -- > 2.20.1 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev