Thanks!
Quoting Timothy Arceri (2019-04-25 15:36:44) > On 26/4/19 6:50 am, Dylan Baker wrote: > > 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? > > Looks good to me. Thanks! > > > > > 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