Re: [Mesa-dev] [PATCH] nir: Look into uniform structs for samplers when counting num_textures.

[Eric Anholt]

Eric Anholt  writes:

> mesa/st decides whether to update samplers after a program change based on
> whether num_textures is nonzero.  By not counting samplers in a uniform
> struct, we would segfault in
> KHR-GLES3.shaders.struct.uniform.sampler_vertex if it was run in the same
> context after a non-vertex-shader-uniform testcase (as is the case during
> a full conformance run).
>
> v2: Implement using two separate pure functions instead of updating
> pointers.

Jason, did you like this version?


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] nir: Look into uniform structs for samplers when counting num_textures.

[Eric Anholt]

mesa/st decides whether to update samplers after a program change based on
whether num_textures is nonzero.  By not counting samplers in a uniform
struct, we would segfault in
KHR-GLES3.shaders.struct.uniform.sampler_vertex if it was run in the same
context after a non-vertex-shader-uniform testcase (as is the case during
a full conformance run).

v2: Implement using two separate pure functions instead of updating
pointers.
---
 src/compiler/nir/nir_gather_info.c | 56 +++---
 1 file changed, 44 insertions(+), 12 deletions(-)

diff --git a/src/compiler/nir/nir_gather_info.c 
b/src/compiler/nir/nir_gather_info.c
index 5530009255d7..95b4456e5b19 100644
--- a/src/compiler/nir/nir_gather_info.c
+++ b/src/compiler/nir/nir_gather_info.c
@@ -350,24 +350,56 @@ gather_info_block(nir_block *block, nir_shader *shader)
}
 }
 
+static unsigned
+glsl_type_get_sampler_count(const struct glsl_type *type)
+{
+   if (glsl_type_is_array(type)) {
+  return (glsl_get_aoa_size(type) *
+  glsl_type_get_sampler_count(glsl_without_array(type)));
+   }
+
+   if (glsl_type_is_struct(type)) {
+  unsigned count = 0;
+  for (int i = 0; i < glsl_get_length(type); i++)
+ count += glsl_type_get_sampler_count(glsl_get_struct_field(type, i));
+  return count;
+   }
+
+   if (glsl_type_is_sampler(type))
+  return 1;
+
+   return 0;
+}
+
+static unsigned
+glsl_type_get_image_count(const struct glsl_type *type)
+{
+   if (glsl_type_is_array(type)) {
+  return (glsl_get_aoa_size(type) *
+  glsl_type_get_image_count(glsl_without_array(type)));
+   }
+
+   if (glsl_type_is_struct(type)) {
+  unsigned count = 0;
+  for (int i = 0; i < glsl_get_length(type); i++)
+ count += glsl_type_get_image_count(glsl_get_struct_field(type, i));
+  return count;
+   }
+
+   if (glsl_type_is_image(type))
+  return 1;
+
+   return 0;
+}
+
 void
 nir_shader_gather_info(nir_shader *shader, nir_function_impl *entrypoint)
 {
shader->info.num_textures = 0;
shader->info.num_images = 0;
nir_foreach_variable(var, >uniforms) {
-  const struct glsl_type *type = var->type;
-  unsigned count = 1;
-  if (glsl_type_is_array(type)) {
- count = glsl_get_aoa_size(type);
- type = glsl_without_array(type);
-  }
-
-  if (glsl_type_is_image(type)) {
- shader->info.num_images += count;
-  } else if (glsl_type_is_sampler(type)) {
- shader->info.num_textures += count;
-  }
+  shader->info.num_textures += glsl_type_get_sampler_count(var->type);
+  shader->info.num_images += glsl_type_get_image_count(var->type);
}
 
shader->info.inputs_read = 0;
-- 
2.17.0

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

Re: [Mesa-dev] [PATCH] nir: Look into uniform structs for samplers when counting num_textures.

[Jason Ekstrand]

Doesn't st/mesa have a pass for getting rid if the structs?

On Mon, Apr 2, 2018 at 11:13 AM, Eric Anholt  wrote:

> mesa/st decides whether to update samplers after a program change based on
> whether num_textures is nonzero.  By not counting samplers in a uniform
> struct, we would segfault in
> KHR-GLES3.shaders.struct.uniform.sampler_vertex if it was run in the same
> context after a non-vertex-shader-uniform testcase (as is the case during
> a full conformance run).
> ---
>  src/compiler/nir/nir_gather_info.c | 32 
>  1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/src/compiler/nir/nir_gather_info.c
> b/src/compiler/nir/nir_gather_info.c
> index 5530009255d7..63783f9bc6a2 100644
> --- a/src/compiler/nir/nir_gather_info.c
> +++ b/src/compiler/nir/nir_gather_info.c
> @@ -350,24 +350,32 @@ gather_info_block(nir_block *block, nir_shader
> *shader)
> }
>  }
>
> +static void
> +nir_gather_uniform_info(nir_shader *shader, const struct glsl_type *type,
> +int count)
>
+{
> +   if (glsl_type_is_array(type)) {
> +  nir_gather_uniform_info(shader, glsl_without_array(type),
> +  count * glsl_get_aoa_size(type));
> +   } else if (glsl_type_is_struct(type)) {
> +  for (int i = 0; i < glsl_get_length(type); i++) {
> + nir_gather_uniform_info(shader, glsl_get_struct_field(type, i),
> + count);
>

The interaction between in and out parameters is a bit weird here.  Why not
just have a simpler helper:

unsigned
glsl_type_get_image_count(const struct glsl_type *type)
{
   if (glsl_type_is_array(type)) {
  return glsl_type_get_image_count(glsl_without_array(type)) *
glsl_get_aoa_size(type);
   } else if (glsl_type_is_struct(type)) {
  unsigned count = 0;
  for (int i = 0; i < glsl_get_length(type); i++)
 count += glsl_type_get_image_count(glsl_get_struct_field(type, i));
  return count;
   } else if (glsl_type_is_image(type) {
  return 1;
   } else {
  return 0;
}

and another one for samplers.  Yes, it's a bit more code but the new
functions are pure which is nice.

--Jason


> +  }
> +   } else if (glsl_type_is_image(type)) {
> +  shader->info.num_images += count;
> +   } else if (glsl_type_is_sampler(type)) {
> +  shader->info.num_textures += count;
> +   }
> +}
> +
>  void
>  nir_shader_gather_info(nir_shader *shader, nir_function_impl *entrypoint)
>  {
> shader->info.num_textures = 0;
> shader->info.num_images = 0;
> nir_foreach_variable(var, >uniforms) {
> -  const struct glsl_type *type = var->type;
> -  unsigned count = 1;
> -  if (glsl_type_is_array(type)) {
> - count = glsl_get_aoa_size(type);
> - type = glsl_without_array(type);
> -  }
> -
> -  if (glsl_type_is_image(type)) {
> - shader->info.num_images += count;
> -  } else if (glsl_type_is_sampler(type)) {
> - shader->info.num_textures += count;
> -  }
> +  nir_gather_uniform_info(shader, var->type, 1);
> }
>
> shader->info.inputs_read = 0;
> --
> 2.16.2
>
> ___
> 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

[Mesa-dev] [PATCH] nir: Look into uniform structs for samplers when counting num_textures.

[Eric Anholt]

mesa/st decides whether to update samplers after a program change based on
whether num_textures is nonzero.  By not counting samplers in a uniform
struct, we would segfault in
KHR-GLES3.shaders.struct.uniform.sampler_vertex if it was run in the same
context after a non-vertex-shader-uniform testcase (as is the case during
a full conformance run).
---
 src/compiler/nir/nir_gather_info.c | 32 
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/compiler/nir/nir_gather_info.c 
b/src/compiler/nir/nir_gather_info.c
index 5530009255d7..63783f9bc6a2 100644
--- a/src/compiler/nir/nir_gather_info.c
+++ b/src/compiler/nir/nir_gather_info.c
@@ -350,24 +350,32 @@ gather_info_block(nir_block *block, nir_shader *shader)
}
 }
 
+static void
+nir_gather_uniform_info(nir_shader *shader, const struct glsl_type *type,
+int count)
+{
+   if (glsl_type_is_array(type)) {
+  nir_gather_uniform_info(shader, glsl_without_array(type),
+  count * glsl_get_aoa_size(type));
+   } else if (glsl_type_is_struct(type)) {
+  for (int i = 0; i < glsl_get_length(type); i++) {
+ nir_gather_uniform_info(shader, glsl_get_struct_field(type, i),
+ count);
+  }
+   } else if (glsl_type_is_image(type)) {
+  shader->info.num_images += count;
+   } else if (glsl_type_is_sampler(type)) {
+  shader->info.num_textures += count;
+   }
+}
+
 void
 nir_shader_gather_info(nir_shader *shader, nir_function_impl *entrypoint)
 {
shader->info.num_textures = 0;
shader->info.num_images = 0;
nir_foreach_variable(var, >uniforms) {
-  const struct glsl_type *type = var->type;
-  unsigned count = 1;
-  if (glsl_type_is_array(type)) {
- count = glsl_get_aoa_size(type);
- type = glsl_without_array(type);
-  }
-
-  if (glsl_type_is_image(type)) {
- shader->info.num_images += count;
-  } else if (glsl_type_is_sampler(type)) {
- shader->info.num_textures += count;
-  }
+  nir_gather_uniform_info(shader, var->type, 1);
}
 
shader->info.inputs_read = 0;
-- 
2.16.2

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