On 08/08/2017 07:34 AM, Timothy Arceri wrote:
On 01/08/17 19:37, Timothy Arceri wrote:
On 01/08/17 18:07, Samuel Pitoiset wrote:
Don't you think it's just safer to revert the bad commit for 17.2 and fix it later on?

I'm not overly worried. If you really want to go that way we can, but I don't think it's necessary.

So how do we move forward on this? Can we have the revert applied to 17.2 and commit this to master?

Sorry, missed that. I would prefer to revert yes, but opinions are welcome. :-)


Ccing Emil for answers :)



On 08/01/2017 09:35 AM, Timothy Arceri wrote:
When generation the storage offset for struct members we need
to skip opaque types as they no longer have backing storage.

Fixes: fcbb93e86024 ("mesa: stop assigning unused storage for non-bindless opaque types")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101983
---
  src/mesa/Makefile.sources                  |   2 +
  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  16 ++++-
src/mesa/state_tracker/st_glsl_types.cpp | 105 +++++++++++++++++++++++++++++
  src/mesa/state_tracker/st_glsl_types.h     |  44 ++++++++++++
  4 files changed, 165 insertions(+), 2 deletions(-)
  create mode 100644 src/mesa/state_tracker/st_glsl_types.cpp
  create mode 100644 src/mesa/state_tracker/st_glsl_types.h

diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
index 97c8287acb..2c557c3952 100644
--- a/src/mesa/Makefile.sources
+++ b/src/mesa/Makefile.sources
@@ -500,20 +500,22 @@ STATETRACKER_FILES = \
      state_tracker/st_extensions.c \
      state_tracker/st_extensions.h \
      state_tracker/st_format.c \
      state_tracker/st_format.h \
      state_tracker/st_gen_mipmap.c \
      state_tracker/st_gen_mipmap.h \
      state_tracker/st_gl_api.h \
      state_tracker/st_glsl_to_nir.cpp \
      state_tracker/st_glsl_to_tgsi.cpp \
      state_tracker/st_glsl_to_tgsi.h \
+-       state_tracker/st_glsl_types.cpp \
+-       state_tracker/st_glsl_types.h \
      state_tracker/st_manager.c \
      state_tracker/st_manager.h \
      state_tracker/st_mesa_to_tgsi.c \
      state_tracker/st_mesa_to_tgsi.h \
      state_tracker/st_nir.h \
      state_tracker/st_nir_lower_builtin.c \
      state_tracker/st_nir_lower_tex_src_plane.c \
      state_tracker/st_pbo.c \
      state_tracker/st_pbo.h \
      state_tracker/st_program.c \
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 2fca398a51..54a749d388 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -42,20 +42,21 @@
  #include "main/shaderapi.h"
  #include "main/shaderimage.h"
  #include "program/prog_instruction.h"
  #include "pipe/p_context.h"
  #include "pipe/p_screen.h"
  #include "tgsi/tgsi_ureg.h"
  #include "tgsi/tgsi_info.h"
  #include "util/u_math.h"
  #include "util/u_memory.h"
+#include "st_glsl_types.h"
  #include "st_program.h"
  #include "st_mesa_to_tgsi.h"
  #include "st_format.h"
  #include "st_nir.h"
  #include "st_shader_cache.h"
  #include "util/hash_table.h"
  #include <algorithm>
  #define PROGRAM_ANY_CONST ((1 << PROGRAM_STATE_VAR) |    \
@@ -2825,22 +2826,30 @@ shrink_array_declarations(struct inout_decl *decls, unsigned count,
              *usage_mask |= BITFIELD64_BIT(decl->mesa_index + j);
        }
     }
  }
  void
  glsl_to_tgsi_visitor::visit(ir_dereference_array *ir)
  {
     ir_constant *index;
     st_src_reg src;
-   int element_size = type_size(ir->type);
     bool is_2D = false;
+   ir_variable *var = ir->variable_referenced();
+
+   /* We only need the logic provided by st_glsl_storage_type_size()
+ * for arrays of structs. Indirect sampler and image indexing is handled
+    * elsewhere.
+    */
+   int element_size = ir->type->without_array()->is_record() ?
+      st_glsl_storage_type_size(ir->type, var->data.bindless) :
+      type_size(ir->type);
     index = ir->array_index->constant_expression_value();
     ir->array->accept(this);
     src = this->result;
     if (ir->array->ir_type != ir_type_dereference_array) {
        switch (this->prog->Target) {
        case GL_TESS_CONTROL_PROGRAM_NV:
is_2D = (src.file == PROGRAM_INPUT || src.file == PROGRAM_OUTPUT) && @@ -2916,28 +2925,31 @@ glsl_to_tgsi_visitor::visit(ir_dereference_array *ir)
     src.type = ir->type->base_type;
     this->result = src;
  }
  void
  glsl_to_tgsi_visitor::visit(ir_dereference_record *ir)
  {
     unsigned int i;
     const glsl_type *struct_type = ir->record->type;
+   ir_variable *var = ir->record->variable_referenced();
     int offset = 0;
     ir->record->accept(this);
+   assert(var);
     for (i = 0; i < struct_type->length; i++) {
if (strcmp(struct_type->fields.structure[i].name, ir->field) == 0)
           break;
-      offset += type_size(struct_type->fields.structure[i].type);
+ const glsl_type *member_type = struct_type->fields.structure[i].type; + offset += st_glsl_storage_type_size(member_type, var->data.bindless);
     }
/* If the type is smaller than a vec4, replicate the last channel out. */
     if (ir->type->is_scalar() || ir->type->is_vector())
this->result.swizzle = swizzle_for_size(ir->type->vector_elements);
     else
        this->result.swizzle = SWIZZLE_NOOP;
     this->result.index += offset;
     this->result.type = ir->type->base_type;
diff --git a/src/mesa/state_tracker/st_glsl_types.cpp b/src/mesa/state_tracker/st_glsl_types.cpp
new file mode 100644
index 0000000000..50936025d9
--- /dev/null
+++ b/src/mesa/state_tracker/st_glsl_types.cpp
@@ -0,0 +1,105 @@
+/*
+ * Copyright (C) 2005-2007  Brian Paul   All Rights Reserved.
+ * Copyright (C) 2008  VMware, Inc.   All Rights Reserved.
+ * Copyright © 2010 Intel Corporation
+ * Copyright © 2011 Bryan Cain
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include "st_glsl_types.h"
+
+/**
+ * Returns the number of places to offset the uniform index, given the type of + * a struct member. We use this because samplers and images have backing
+ * storeage only when they are bindless.
+ */
+int
+st_glsl_storage_type_size(const struct glsl_type *type, bool is_bindless)
+{
+   unsigned int i;
+   int size;
+
+   switch (type->base_type) {
+   case GLSL_TYPE_UINT:
+   case GLSL_TYPE_INT:
+   case GLSL_TYPE_FLOAT:
+   case GLSL_TYPE_BOOL:
+      if (type->is_matrix()) {
+         return type->matrix_columns;
+      } else {
+         /* Regardless of size of vector, it gets a vec4. This is bad
+ * packing for things like floats, but otherwise arrays become a + * mess. Hopefully a later pass over the code can pack scalars
+          * down if appropriate.
+          */
+         return 1;
+      }
+      break;
+   case GLSL_TYPE_DOUBLE:
+      if (type->is_matrix()) {
+         if (type->vector_elements <= 2)
+            return type->matrix_columns;
+         else
+            return type->matrix_columns * 2;
+      } else {
+         /* For doubles if we have a double or dvec2 they fit in one
+          * vec4, else they need 2 vec4s.
+          */
+         if (type->vector_elements <= 2)
+            return 1;
+         else
+            return 2;
+      }
+      break;
+   case GLSL_TYPE_UINT64:
+   case GLSL_TYPE_INT64:
+      if (type->vector_elements <= 2)
+         return 1;
+      else
+         return 2;
+   case GLSL_TYPE_ARRAY:
+      assert(type->length > 0);
+ return st_glsl_storage_type_size(type->fields.array, is_bindless) *
+         type->length;
+   case GLSL_TYPE_STRUCT:
+      size = 0;
+      for (i = 0; i < type->length; i++) {
+ size += st_glsl_storage_type_size(type->fields.structure[i].type,
+                                               is_bindless);
+      }
+      return size;
+   case GLSL_TYPE_SAMPLER:
+   case GLSL_TYPE_IMAGE:
+      if (!is_bindless)
+         return 0;
+      /* fall through */
+   case GLSL_TYPE_SUBROUTINE:
+      return 1;
+   case GLSL_TYPE_ATOMIC_UINT:
+   case GLSL_TYPE_INTERFACE:
+   case GLSL_TYPE_VOID:
+   case GLSL_TYPE_ERROR:
+   case GLSL_TYPE_FUNCTION:
+      assert(!"Invalid type in type_size");
+      break;
+   }
+   return 0;
+}
diff --git a/src/mesa/state_tracker/st_glsl_types.h b/src/mesa/state_tracker/st_glsl_types.h
new file mode 100644
index 0000000000..915816d1fa
--- /dev/null
+++ b/src/mesa/state_tracker/st_glsl_types.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2005-2007  Brian Paul   All Rights Reserved.
+ * Copyright (C) 2008  VMware, Inc.   All Rights Reserved.
+ * Copyright © 2010 Intel Corporation
+ * Copyright © 2011 Bryan Cain
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __ST_GLSL_TYPES_H__
+#define __ST_GLSL_TYPES_H__
+
+#include "compiler/glsl_types.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+int st_glsl_storage_type_size(const struct glsl_type *type,
+                              bool is_bindless);
+
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __ST_GLSL_TYPES_H__ */

_______________________________________________
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

Reply via email to