Re: [Mesa-dev] [PATCH 2/5] glsl: add support for the imageSize builtin

[Martin Peres]

On 11/08/15 19:52, Ilia Mirkin wrote:
On Tue, Aug 11, 2015 at 12:43 PM, Martin Peres
<martin.pe...@linux.intel.com> wrote:
The code is heavily inspired from Francisco Jerez's code supporting the
image_load_store extension.
I moved out image_types[] out of builtin_builder::add_image_function to
share it with the newly-added builtin_builder::add_image_size_functions.

Backends willing to support this builtin should handle
__intrinsic_image_size.

Signed-off-by: Martin Peres <martin.pe...@linux.intel.com>
---
  src/glsl/builtin_functions.cpp | 165 ++++++++++++++++++++++++++++++-----------
  1 file changed, 122 insertions(+), 43 deletions(-)

diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
index 2175c66..e8344f0 100644
--- a/src/glsl/builtin_functions.cpp
+++ b/src/glsl/builtin_functions.cpp
@@ -399,6 +399,13 @@ shader_image_load_store(const _mesa_glsl_parse_state 
*state)
  }

  static bool
+shader_image_size(const _mesa_glsl_parse_state *state)
+{
+   return (state->is_version(430, 0) ||
Isn't it also available in GLSL ES 3.10? i.e. this should be
state->is_version(430, 310). Or is the remaining image stuff not quite
ready for ES 3.10?

Oh, right!  ES 3.10 should be OK, but I will check in details tomorrow. 
I wrote this patch series months ago so it is good to have a look at the 
spec again anyway.

Thanks!


+           state->ARB_shader_image_size_enable);
+}
+
+static bool
  gs_streams(const _mesa_glsl_parse_state *state)
  {
     return gpu_shader5(state) && gs_only(state);
@@ -502,8 +509,8 @@ private:
     };

     /**
-    * Create a new image built-in function for all known image types.
-    * \p flags is a bitfield of \c image_function_flags flags.
+    * Create a new image built-in function for image load/store for all known
+    * image types. \p flags is a bitfield of \c image_function_flags flags.
      */
     void add_image_function(const char *name,
                             const char *intrinsic_name,
@@ -511,14 +518,24 @@ private:
                             unsigned flags);

     /**
-    * Create new functions for all known image built-ins and types.
-    * If \p glsl is \c true, use the GLSL built-in names and emit code
-    * to call into the actual compiler intrinsic.  If \p glsl is
+    * Create a new image built-in function for image load/store for all known
+    * image types. If \p glsl is \c true, use the GLSL built-in names and emit
+    * code to call into the actual compiler intrinsic.  If \p glsl is
      * false, emit a function prototype with no body for each image
      * intrinsic name.
      */
     void add_image_functions(bool glsl);

+   /**
+    * Create the imageSize built-in function for all known image built-ins and
+    * types. If \p glsl is \c true, use the GLSL built-in names and emit code
+    * to call into the actual compiler intrinsic.  If \p glsl is
+    * false, emit a function prototype with no body for each image
+    * intrinsic name.
+    */
+   void add_image_size_functions(const char *name, const char *intrinsic_name,
+                                 unsigned flags);
+
     ir_function_signature *new_sig(const glsl_type *return_type,
                                    builtin_available_predicate avail,
                                    int num_params, ...);
@@ -718,6 +735,10 @@ private:
     ir_function_signature *_memory_barrier(
        builtin_available_predicate avail);

+   ir_function_signature *_image_size(const glsl_type *image_type,
+                                      const char *intrinsic_name,
+                                      unsigned flags);
+
  #undef B0
  #undef B1
  #undef B2
@@ -2549,53 +2570,54 @@ builtin_builder::add_function(const char *name, ...)
     shader->symbols->add_function(f);
  }

+static const glsl_type *const image_types[] = {
+   glsl_type::image1D_type,
+   glsl_type::image2D_type,
+   glsl_type::image3D_type,
+   glsl_type::image2DRect_type,
+   glsl_type::imageCube_type,
+   glsl_type::imageBuffer_type,
+   glsl_type::image1DArray_type,
+   glsl_type::image2DArray_type,
+   glsl_type::imageCubeArray_type,
+   glsl_type::image2DMS_type,
+   glsl_type::image2DMSArray_type,
+   glsl_type::iimage1D_type,
+   glsl_type::iimage2D_type,
+   glsl_type::iimage3D_type,
+   glsl_type::iimage2DRect_type,
+   glsl_type::iimageCube_type,
+   glsl_type::iimageBuffer_type,
+   glsl_type::iimage1DArray_type,
+   glsl_type::iimage2DArray_type,
+   glsl_type::iimageCubeArray_type,
+   glsl_type::iimage2DMS_type,
+   glsl_type::iimage2DMSArray_type,
+   glsl_type::uimage1D_type,
+   glsl_type::uimage2D_type,
+   glsl_type::uimage3D_type,
+   glsl_type::uimage2DRect_type,
+   glsl_type::uimageCube_type,
+   glsl_type::uimageBuffer_type,
+   glsl_type::uimage1DArray_type,
+   glsl_type::uimage2DArray_type,
+   glsl_type::uimageCubeArray_type,
+   glsl_type::uimage2DMS_type,
+   glsl_type::uimage2DMSArray_type
+};
+
  void
  builtin_builder::add_image_function(const char *name,
                                      const char *intrinsic_name,
                                      unsigned num_arguments,
                                      unsigned flags)
  {
-   static const glsl_type *const types[] = {
-      glsl_type::image1D_type,
-      glsl_type::image2D_type,
-      glsl_type::image3D_type,
-      glsl_type::image2DRect_type,
-      glsl_type::imageCube_type,
-      glsl_type::imageBuffer_type,
-      glsl_type::image1DArray_type,
-      glsl_type::image2DArray_type,
-      glsl_type::imageCubeArray_type,
-      glsl_type::image2DMS_type,
-      glsl_type::image2DMSArray_type,
-      glsl_type::iimage1D_type,
-      glsl_type::iimage2D_type,
-      glsl_type::iimage3D_type,
-      glsl_type::iimage2DRect_type,
-      glsl_type::iimageCube_type,
-      glsl_type::iimageBuffer_type,
-      glsl_type::iimage1DArray_type,
-      glsl_type::iimage2DArray_type,
-      glsl_type::iimageCubeArray_type,
-      glsl_type::iimage2DMS_type,
-      glsl_type::iimage2DMSArray_type,
-      glsl_type::uimage1D_type,
-      glsl_type::uimage2D_type,
-      glsl_type::uimage3D_type,
-      glsl_type::uimage2DRect_type,
-      glsl_type::uimageCube_type,
-      glsl_type::uimageBuffer_type,
-      glsl_type::uimage1DArray_type,
-      glsl_type::uimage2DArray_type,
-      glsl_type::uimageCubeArray_type,
-      glsl_type::uimage2DMS_type,
-      glsl_type::uimage2DMSArray_type
-   };
     ir_function *f = new(mem_ctx) ir_function(name);

-   for (unsigned i = 0; i < ARRAY_SIZE(types); ++i) {
-      if (types[i]->sampler_type != GLSL_TYPE_FLOAT ||
+   for (unsigned i = 0; i < ARRAY_SIZE(image_types); ++i) {
+      if (image_types[i]->sampler_type != GLSL_TYPE_FLOAT ||
            (flags & IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE))
-         f->add_signature(_image(types[i], intrinsic_name,
+         f->add_signature(_image(image_types[i], intrinsic_name,
                                   num_arguments, flags));
     }

@@ -2603,6 +2625,20 @@ builtin_builder::add_image_function(const char *name,
  }

  void
+builtin_builder::add_image_size_functions(const char *name,
+                                const char *intrinsic_name,
+                                unsigned flags)
+{
+   ir_function *f = new(mem_ctx) ir_function(name);
+
+   for (unsigned i = 0; i < ARRAY_SIZE(image_types); ++i) {
+         f->add_signature(_image_size(image_types[i], intrinsic_name, flags));
3-space indent please.

+   }
+
+   shader->symbols->add_function(f);
+}
+
+void
  builtin_builder::add_image_functions(bool glsl)
  {
     const unsigned flags = (glsl ? IMAGE_FUNCTION_EMIT_STUB : 0);
@@ -2645,6 +2681,10 @@ builtin_builder::add_image_functions(bool glsl)
     add_image_function((glsl ? "imageAtomicCompSwap" :
                         "__intrinsic_image_atomic_comp_swap"),
                        "__intrinsic_image_atomic_comp_swap", 2, flags);
+
+   add_image_size_functions((glsl ? "imageSize" :
+                             "__intrinsic_image_size"),
+                            "__intrinsic_image_size", flags);
  }

  ir_variable *
@@ -4864,6 +4904,45 @@ 
builtin_builder::_memory_barrier(builtin_available_predicate avail)
     return sig;
  }

+ir_function_signature *
+builtin_builder::_image_size(const glsl_type *image_type,
+                             const char *intrinsic_name, unsigned flags)
+{
+   const glsl_type *ret_type = glsl_type::get_instance(
+            image_type->sampler_type, image_type->coordinate_components(), 1);
errr.... wha? Shouldn't this be glsl_type::get_instance(GLSL_TYPE_INT,
image_type->coordinate_components(), 1) ? Otherwise a image2D will be
a vec2 instead of an ivec2.

Also what about cube? I assume that it has 3 coordinate components,
but it's supposed to only return an ivec2.

+
+   ir_variable *image = in_var(image_type, "image");
+   ir_function_signature *sig = new_sig(ret_type, shader_image_size, 1, image);
+
+   if (flags & IMAGE_FUNCTION_EMIT_STUB) {
+      ir_factory body(&sig->body, mem_ctx);
+      ir_function *f = shader->symbols->get_function(intrinsic_name);
+
+      ir_variable *ret_val = body.make_temp(sig->return_type, "_ret_val");
+      body.emit(call(f, ret_val, sig->parameters));
+      body.emit(ret(ret_val));
+
+      sig->is_defined = true;
+   } else {
+      sig->is_intrinsic = true;
+   }
+
+   /* Set the maximal set of qualifiers allowed for this image
+    * built-in.  Function calls with arguments having fewer
+    * qualifiers than present in the prototype are allowed by the
+    * spec, but not with more, i.e. this will make the compiler
+    * accept everything that needs to be accepted, and reject cases
+    * like loads from write-only or stores to read-only images.
+    */
+   image->data.image_read_only = true;
+   image->data.image_write_only = true;
+   image->data.image_coherent = true;
+   image->data.image_volatile = true;
+   image->data.image_restrict = true;
+
+   return sig;
+}
+
  /** @} */

  
/******************************************************************************/
--
2.5.0

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

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