On 23/09/17 04:33, Kenneth Graunke wrote:
On Tuesday, September 12, 2017 4:37:30 PM PDT Timothy Arceri wrote:
---
  src/compiler/nir/nir.h | 31 +++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index fab2110f619..e52a1006896 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -351,6 +351,37 @@ typedef struct nir_variable {
  #define nir_foreach_variable_safe(var, var_list) \
     foreach_list_typed_safe(nir_variable, var, node, var_list)
+/**
+ * Returns the bits in the inputs_read, outputs_written, or
+ * system_values_read bitfield corresponding to this variable.
+ */
+static inline uint64_t
+nir_variable_get_io_mask(nir_variable *var, gl_shader_stage stage)
+{
+   /* TODO: add support for tess patches */
+   if (var->data.patch || var->data.location < 0)
+      return 0;
+
+   assert(var->data.mode == nir_var_shader_in ||
+          var->data.mode == nir_var_shader_out ||
+          var->data.mode == nir_var_system_value);
+   assert(var->data.location >= 0);
+
+   const struct glsl_type *var_type = var->type;
+   if ((var->data.mode == nir_var_shader_in &&
+        (stage == MESA_SHADER_GEOMETRY ||
+         stage == MESA_SHADER_TESS_CTRL ||
+         stage == MESA_SHADER_TESS_EVAL)) ||
+       (var->data.mode == nir_var_shader_out &&
+        stage == MESA_SHADER_TESS_CTRL)) {
+      if (glsl_type_is_array(var_type))
+         var_type = glsl_get_array_element(var_type);

You can just do

    if (nir_is_per_vertex_io(var, stage)) {
       assert(glsl_type_is_array(type));
       type = glsl_get_array_element(type);
    }

Right, thanks :)


+   }
+
+   unsigned slots = glsl_count_attribute_slots(var_type, false);
+   return ((1ull << slots) - 1) << var->data.location;
+}
+
  static inline bool
  nir_variable_is_global(const nir_variable *var)
  {


This is duplicating the code in nir_gather_info.c pretty hard, but it
doesn't handle patch variables, or scalar arrays with var->data.compact
set (i.e. gl_ClipDistance as float[8] packed as if it were vec4[2]).

We don't try to remove builtins so var->data.compact shouldn't be a problem right?


Would it make sense to wrap or refactor set_io_mask()?


Maybe, I'm not sure. There is a bunch of stuff in nir_gather_info.c we don't really need just yet. For example we currently make no attempt to remove components from partially used arrays.

I'd really like to land this first, which with your suggestion above is really a very small amount of code. From here I'm working on a NIR vector splitting pass and small update to this series to handle removal per component rather than per location. Initial results (take with a gain of salt as I still have a handful of piglit issues) are looking very promising.

total instructions in shared programs: 13190730 -> 13110599 (-0.61%)
instructions in affected programs: 2110903 -> 2030772 (-3.80%)
helped: 14043
HURT: 486

total cycles in shared programs: 541148990 -> 540561942 (-0.11%)
cycles in affected programs: 290344296 -> 289757248 (-0.20%)
helped: 23418
HURT: 11623

Once I write the NIR packing pass this should get even better.

I'd really like it if we could just land this patch as is then come back and improve things once a more full linking implementation is in place rather than trying to get it perfect in one step. Does that seem reasonable?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to