On 12/04/17 19:49, Samuel Pitoiset wrote:


On 04/12/2017 02:04 AM, Timothy Arceri wrote:


On 12/04/17 02:48, Samuel Pitoiset wrote:
At this point, there is enough information to take the decision
to replace a sampler/image type by the corresponding bindless one.

Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
---
 src/compiler/glsl/ast.h          | 10 ++++-
 src/compiler/glsl/ast_to_hir.cpp | 93
++++++++++++++++++++++++++++++++++++++--
 2 files changed, 98 insertions(+), 5 deletions(-)

diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index 9327e03979..2a2dd4bcd9 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -894,8 +894,14 @@ public:
    }

    const struct glsl_type *glsl_type(const char **name,
-                     struct _mesa_glsl_parse_state *state)
-      const;
+                                     struct _mesa_glsl_parse_state
*state,
+                                     bool is_interface_block =
false) const;
+
+   bool is_temporary_storage() const;
+
+   bool is_bindless_type(const struct glsl_type *type,
+                         struct _mesa_glsl_parse_state *state,
+                         bool is_interface_block) const;

    ast_type_qualifier qualifier;
    ast_type_specifier *specifier;
diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index 5043d6a574..4a0f3fe315 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -2646,9 +2646,96 @@ select_gles_precision(unsigned qual_precision,

 const glsl_type *
 ast_fully_specified_type::glsl_type(const char **name,
-                                    struct _mesa_glsl_parse_state
*state) const
+                                    struct _mesa_glsl_parse_state
*state,
+                                    bool is_interface_block) const
 {
-   return this->specifier->glsl_type(name, state);
+   const struct glsl_type *type;
+
+   type = this->specifier->glsl_type(name, state);
+   if (!type)
+      return NULL;
+
+   /* Bindless sampler/image types are replaced on-the-fly if needed as
+    * specified by ARB_bindless_texture. */
+   if (state->has_bindless()) {

Can we simplify this to:

if (state->has_bindless() &&
     is_bindless_type(type, state, is_interface_block)) {

And drop the two calls below. By now we should have already thrown an
error if the bindless qualifiers were not applied to an image/sampler
right?

Yes, this can be simplied to:

if (state->has_bindless() &&
    is_bindless_type(type, state, is_interface_block)) {
  if (type->is_sampler())
    type = get_sampler_instance(...);
  if (type->is_image())
    type = get_image_instance(...);
}

But, we need to check if the type is sampler or image because of the
global layout qualifier.


Yep, that's what I meant. Thanks :)




+      if (type->is_sampler()) {
+         if (is_bindless_type(type, state, is_interface_block)) {
+            type = glsl_type::get_sampler_instance(
+                     (glsl_sampler_dim)type->sampler_dimensionality,
+                     type->sampler_shadow,
+                     type->sampler_array,
+                     (glsl_base_type)type->sampled_type,
+                     true);
+         }
+      }
+      if (type->is_image()) {
+         if (is_bindless_type(type, state, is_interface_block)) {
+            type = glsl_type::get_image_instance(
+                     (glsl_sampler_dim)type->sampler_dimensionality,
+                     type->sampler_array,
+                     (glsl_base_type)type->sampled_type,
+                     true);
+         }
+      }
+   }
+
+   return type;
+}
+
+bool
+ast_fully_specified_type::is_temporary_storage() const
+{
+   return (!qualifier.flags.q.in &&
+           !qualifier.flags.q.out &&
+           !qualifier.flags.q.attribute &&
+           !qualifier.flags.q.varying &&
+           !qualifier.flags.q.uniform &&
+           !qualifier.flags.q.buffer &&
+           !qualifier.flags.q.shared_storage);
+}

Just noticed, this should be moved to
ast_type_qualifier::is_temporary_storage().

Will do in a separate patch just before this one.


Cool. I was going to ask about this but seem to have missed it.



+
+bool
+ast_fully_specified_type::is_bindless_type(const struct glsl_type
*type,
+                                           struct
_mesa_glsl_parse_state *state,
+                                           bool is_interface_block)
const
+{
+   /* The bindless_sampler (and respectively bindless_image) layout
qualifiers
+    * can be set as local scope, as well as at global scope. */
+   if (qualifier.flags.q.bindless_sampler ||
+       qualifier.flags.q.bindless_image ||
+       state->bindless_sampler_specified ||
+       state->bindless_image_specified)
+      return true;
+
+   /* The ARB_bindless_texture spec says:
+    *
+    * "Modify Section 4.3.7, Interface Blocks, p. 38"
+    *
+    * "(remove the following bullet from the last list on p. 39,
thereby
+    *  permitting sampler types in interface blocks; image types are
also
+    *  permitted in blocks by this extension)"
+    *
+    *    * sampler types are not allowed
+    */
+   if (is_interface_block)
+      return true;
+
+   /* The ARB_bindless_texture spec says:
+    *
+    * "Replace Section 4.1.7 (Samplers), p. 25"
+    *
+    * "Samplers may be declared as shader inputs and outputs, as
uniform
+    *  variables, as temporary variables, and as function parameters"
+    *
+    * "Replace Section 4.1.X, (Images)"
+    *
+    * "Images may be declared as shader inputs and outputs, as uniform
+    *  variables, as temporary variables, and as function parameters."
+    */
+   if (qualifier.flags.q.in || qualifier.flags.q.out ||
is_temporary_storage())
+      return true;
+
+   return false;
 }

 /**
@@ -6844,7 +6931,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
                           "embedded structure declarations are not
allowed");

       const glsl_type *decl_type =
-         decl_list->type->glsl_type(& type_name, state);
+         decl_list->type->glsl_type(& type_name, state, is_interface);

This looks like it should be a separate change?

Okay, I can put this into a separate patch before this one as well.


Cool. I just wasn't sure if this was a general fix, or if it was specific to bindless somehow. A separate patch with a description would solve that. Thanks.





       const struct ast_type_qualifier *const qual =
          &decl_list->type->qualifier;

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

Reply via email to