On 11/10/18 1:46 am, Samuel Pitoiset wrote:
We shouldn't try to compact any varyings known as always
active IO, especially XFB outputs. For example, if one
component of an xfb output is also used as input varying
in the next stage, it shouldn't be compacted.

This small helper just marks all XFB varyings as always_active_io
in the consumer to not compact them.

v2: add a little helper

Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
---
  src/compiler/nir/nir_linking_helpers.c | 33 ++++++++++++++++++++++++++
  1 file changed, 33 insertions(+)

diff --git a/src/compiler/nir/nir_linking_helpers.c 
b/src/compiler/nir/nir_linking_helpers.c
index 88014e9a1d..433729bd79 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -480,6 +480,36 @@ compact_components(nir_shader *producer, nir_shader 
*consumer, uint8_t *comps,
                                &producer->info.outputs_read);
  }
+/* Mark XFB varyings as always_active_io in the consumer to not compact them. */
+static void
+link_xfb_varyings(nir_shader *producer, nir_shader *consumer)
+{
+   nir_variable *input_vars[32] = {};

      nir_variable *input_vars[MAX_VARYING] = {};

+
+   nir_foreach_variable(var, &consumer->inputs) {
+      if (var->data.location >= VARYING_SLOT_VAR0 &&
+          var->data.location - VARYING_SLOT_VAR0 < 32) {

          var->data.location - VARYING_SLOT_VAR0 < MAX_VARYING) {


+
+         unsigned location = var->data.location - VARYING_SLOT_VAR0;
+         input_vars[location] = var;
+      }
+   }
+
+   nir_foreach_variable(var, &producer->outputs) {
+      if (var->data.location >= VARYING_SLOT_VAR0 &&
+          var->data.location - VARYING_SLOT_VAR0 < 32) {

          var->data.location - VARYING_SLOT_VAR0 < MAX_VARYING) {

+
+         if (!var->data.explicit_xfb_buffer)
+            continue;
+
+         unsigned location = var->data.location - VARYING_SLOT_VAR0;
+         if (input_vars[location]) {
+            input_vars[location]->data.always_active_io = true;
+         }
+      }
+   }
+}
+
  /* We assume that this has been called more-or-less directly after
   * remove_unused_varyings.  At this point, all of the varyings that we
   * aren't going to be using have been completely removed and the
@@ -501,6 +531,9 @@ nir_compact_varyings(nir_shader *producer, nir_shader 
*consumer,
     uint8_t interp_type[32] = {0};
     uint8_t interp_loc[32] = {0};

Hmm, looks like I should fix up these magic numbers also. I was planning on changing this code when adding patch support (which I still haven't got round to).

+ if (producer->info.has_transform_feedback_varyings)
+      link_xfb_varyings(producer, consumer);

I think you need to expose link_xfb_varyings() externally and call it in radv_link_shaders() before nir_lower_io_arrays_to_elements()

e.g

   if (ordered_shaders[i]->info.has_transform_feedback_varyings)
      nir_link_xfb_varyings(ordered_shaders[i], ordered_shaders[i - 1]);

With these changes this patch is:

Reviewed-by: Timothy Arceri <tarc...@itsqueeze.com>


+
     get_slot_component_masks_and_interp_types(&producer->outputs, comps,
                                               interp_type, interp_loc,
                                               producer->info.stage,

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to