Re: [Mesa-dev] [PATCH v2 08/21] spirv: translate default-block uniforms

[Timothy Arceri]

Reviewed-by: Timothy Arceri 

On 12/05/18 19:40, Alejandro Piñeiro wrote:

From: Nicolai Hähnle 

They are supported by SPIR-V for ARB_gl_spirv.

v2 (changes on top of Nicolai's original patch):
* Handle UniformConstant storage class for uniforms other than
  samplers and images. (Eduardo Lima)
* Handle location decoration also for samplers and images. (Eduardo
  Lima)
* Rebase update (spirv_to_nir options added, logging changes, and
  others) (Alejandro Piñeiro)

Signed-off-by: Nicolai Hähnle 
Signed-off-by: Eduardo Lima 
Signed-off-by: Alejandro Piñeiro 
---
  src/compiler/spirv/vtn_private.h   |  1 +
  src/compiler/spirv/vtn_variables.c | 25 +++--
  2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
index b501bbf9b4a..183024e14f4 100644
--- a/src/compiler/spirv/vtn_private.h
+++ b/src/compiler/spirv/vtn_private.h
@@ -402,6 +402,7 @@ enum vtn_variable_mode {
 vtn_variable_mode_local,
 vtn_variable_mode_global,
 vtn_variable_mode_param,
+   vtn_variable_mode_uniform,
 vtn_variable_mode_ubo,
 vtn_variable_mode_ssbo,
 vtn_variable_mode_push_constant,
diff --git a/src/compiler/spirv/vtn_variables.c 
b/src/compiler/spirv/vtn_variables.c
index fd8ab7f247a..eb8a9ca0084 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1544,8 +1544,11 @@ var_decoration_cb(struct vtn_builder *b, struct 
vtn_value *val, int member,
   vtn_var->mode == vtn_variable_mode_output) {
   is_vertex_input = false;
   location += vtn_var->patch ? VARYING_SLOT_PATCH0 : VARYING_SLOT_VAR0;
-  } else {
- vtn_warn("Location must be on input or output variable");
+  } else if (vtn_var->mode != vtn_variable_mode_uniform &&
+ vtn_var->mode != vtn_variable_mode_sampler &&
+ vtn_var->mode != vtn_variable_mode_image) {
+ vtn_warn("Location must be on input, output, uniform, sampler or "
+  "image variable");
   return;
}
  
@@ -1611,7 +1614,9 @@ vtn_storage_class_to_mode(struct vtn_builder *b,

   mode = vtn_variable_mode_ssbo;
   nir_mode = 0;
} else {
- vtn_fail("Invalid uniform variable type");
+ /* Default-block uniforms, coming from gl_spirv */
+ mode = vtn_variable_mode_uniform;
+ nir_mode = nir_var_uniform;
}
break;
 case SpvStorageClassStorageBuffer:
@@ -1619,15 +1624,13 @@ vtn_storage_class_to_mode(struct vtn_builder *b,
nir_mode = 0;
break;
 case SpvStorageClassUniformConstant:
-  if (glsl_type_is_image(interface_type->type)) {
+  if (glsl_type_is_image(interface_type->type))
   mode = vtn_variable_mode_image;
- nir_mode = nir_var_uniform;
-  } else if (glsl_type_is_sampler(interface_type->type)) {
+  else if (glsl_type_is_sampler(interface_type->type))
   mode = vtn_variable_mode_sampler;
- nir_mode = nir_var_uniform;
-  } else {
- vtn_fail("Invalid uniform constant variable type");
-  }
+  else
+ mode = vtn_variable_mode_uniform;
+  nir_mode = nir_var_uniform;
break;
 case SpvStorageClassPushConstant:
mode = vtn_variable_mode_push_constant;
@@ -1795,11 +1798,13 @@ vtn_create_variable(struct vtn_builder *b, struct 
vtn_value *val,
 case vtn_variable_mode_global:
 case vtn_variable_mode_image:
 case vtn_variable_mode_sampler:
+   case vtn_variable_mode_uniform:
/* For these, we create the variable normally */
var->var = rzalloc(b->shader, nir_variable);
var->var->name = ralloc_strdup(var->var, val->name);
var->var->type = var->type->type;
var->var->data.mode = nir_mode;
+  var->var->data.location = -1;
  
switch (var->mode) {

case vtn_variable_mode_image:


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

Re: [Mesa-dev] [PATCH v2 08/21] spirv: translate default-block uniforms

[Alejandro Piñeiro]

Hi Timothy,

thanks for the review of the patches of this series. This patch and the
squash I have just sent are the only patches pending to be reviewed.

Would you mind to take a look to this patch too?

Thanks in advance


On 12/05/18 11:40, Alejandro Piñeiro wrote:
> From: Nicolai Hähnle 
>
> They are supported by SPIR-V for ARB_gl_spirv.
>
> v2 (changes on top of Nicolai's original patch):
>* Handle UniformConstant storage class for uniforms other than
>  samplers and images. (Eduardo Lima)
>* Handle location decoration also for samplers and images. (Eduardo
>  Lima)
>* Rebase update (spirv_to_nir options added, logging changes, and
>  others) (Alejandro Piñeiro)
>
> Signed-off-by: Nicolai Hähnle 
> Signed-off-by: Eduardo Lima 
> Signed-off-by: Alejandro Piñeiro 
> ---
>  src/compiler/spirv/vtn_private.h   |  1 +
>  src/compiler/spirv/vtn_variables.c | 25 +++--
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_private.h 
> b/src/compiler/spirv/vtn_private.h
> index b501bbf9b4a..183024e14f4 100644
> --- a/src/compiler/spirv/vtn_private.h
> +++ b/src/compiler/spirv/vtn_private.h
> @@ -402,6 +402,7 @@ enum vtn_variable_mode {
> vtn_variable_mode_local,
> vtn_variable_mode_global,
> vtn_variable_mode_param,
> +   vtn_variable_mode_uniform,
> vtn_variable_mode_ubo,
> vtn_variable_mode_ssbo,
> vtn_variable_mode_push_constant,
> diff --git a/src/compiler/spirv/vtn_variables.c 
> b/src/compiler/spirv/vtn_variables.c
> index fd8ab7f247a..eb8a9ca0084 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1544,8 +1544,11 @@ var_decoration_cb(struct vtn_builder *b, struct 
> vtn_value *val, int member,
>   vtn_var->mode == vtn_variable_mode_output) {
>   is_vertex_input = false;
>   location += vtn_var->patch ? VARYING_SLOT_PATCH0 : 
> VARYING_SLOT_VAR0;
> -  } else {
> - vtn_warn("Location must be on input or output variable");
> +  } else if (vtn_var->mode != vtn_variable_mode_uniform &&
> + vtn_var->mode != vtn_variable_mode_sampler &&
> + vtn_var->mode != vtn_variable_mode_image) {
> + vtn_warn("Location must be on input, output, uniform, sampler or "
> +  "image variable");
>   return;
>}
>  
> @@ -1611,7 +1614,9 @@ vtn_storage_class_to_mode(struct vtn_builder *b,
>   mode = vtn_variable_mode_ssbo;
>   nir_mode = 0;
>} else {
> - vtn_fail("Invalid uniform variable type");
> + /* Default-block uniforms, coming from gl_spirv */
> + mode = vtn_variable_mode_uniform;
> + nir_mode = nir_var_uniform;
>}
>break;
> case SpvStorageClassStorageBuffer:
> @@ -1619,15 +1624,13 @@ vtn_storage_class_to_mode(struct vtn_builder *b,
>nir_mode = 0;
>break;
> case SpvStorageClassUniformConstant:
> -  if (glsl_type_is_image(interface_type->type)) {
> +  if (glsl_type_is_image(interface_type->type))
>   mode = vtn_variable_mode_image;
> - nir_mode = nir_var_uniform;
> -  } else if (glsl_type_is_sampler(interface_type->type)) {
> +  else if (glsl_type_is_sampler(interface_type->type))
>   mode = vtn_variable_mode_sampler;
> - nir_mode = nir_var_uniform;
> -  } else {
> - vtn_fail("Invalid uniform constant variable type");
> -  }
> +  else
> + mode = vtn_variable_mode_uniform;
> +  nir_mode = nir_var_uniform;
>break;
> case SpvStorageClassPushConstant:
>mode = vtn_variable_mode_push_constant;
> @@ -1795,11 +1798,13 @@ vtn_create_variable(struct vtn_builder *b, struct 
> vtn_value *val,
> case vtn_variable_mode_global:
> case vtn_variable_mode_image:
> case vtn_variable_mode_sampler:
> +   case vtn_variable_mode_uniform:
>/* For these, we create the variable normally */
>var->var = rzalloc(b->shader, nir_variable);
>var->var->name = ralloc_strdup(var->var, val->name);
>var->var->type = var->type->type;
>var->var->data.mode = nir_mode;
> +  var->var->data.location = -1;
>  
>switch (var->mode) {
>case vtn_variable_mode_image:

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

[Mesa-dev] [PATCH v2 08/21] spirv: translate default-block uniforms

[Alejandro Piñeiro]

From: Nicolai Hähnle 

They are supported by SPIR-V for ARB_gl_spirv.

v2 (changes on top of Nicolai's original patch):
   * Handle UniformConstant storage class for uniforms other than
 samplers and images. (Eduardo Lima)
   * Handle location decoration also for samplers and images. (Eduardo
 Lima)
   * Rebase update (spirv_to_nir options added, logging changes, and
 others) (Alejandro Piñeiro)

Signed-off-by: Nicolai Hähnle 
Signed-off-by: Eduardo Lima 
Signed-off-by: Alejandro Piñeiro 
---
 src/compiler/spirv/vtn_private.h   |  1 +
 src/compiler/spirv/vtn_variables.c | 25 +++--
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
index b501bbf9b4a..183024e14f4 100644
--- a/src/compiler/spirv/vtn_private.h
+++ b/src/compiler/spirv/vtn_private.h
@@ -402,6 +402,7 @@ enum vtn_variable_mode {
vtn_variable_mode_local,
vtn_variable_mode_global,
vtn_variable_mode_param,
+   vtn_variable_mode_uniform,
vtn_variable_mode_ubo,
vtn_variable_mode_ssbo,
vtn_variable_mode_push_constant,
diff --git a/src/compiler/spirv/vtn_variables.c 
b/src/compiler/spirv/vtn_variables.c
index fd8ab7f247a..eb8a9ca0084 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1544,8 +1544,11 @@ var_decoration_cb(struct vtn_builder *b, struct 
vtn_value *val, int member,
  vtn_var->mode == vtn_variable_mode_output) {
  is_vertex_input = false;
  location += vtn_var->patch ? VARYING_SLOT_PATCH0 : VARYING_SLOT_VAR0;
-  } else {
- vtn_warn("Location must be on input or output variable");
+  } else if (vtn_var->mode != vtn_variable_mode_uniform &&
+ vtn_var->mode != vtn_variable_mode_sampler &&
+ vtn_var->mode != vtn_variable_mode_image) {
+ vtn_warn("Location must be on input, output, uniform, sampler or "
+  "image variable");
  return;
   }
 
@@ -1611,7 +1614,9 @@ vtn_storage_class_to_mode(struct vtn_builder *b,
  mode = vtn_variable_mode_ssbo;
  nir_mode = 0;
   } else {
- vtn_fail("Invalid uniform variable type");
+ /* Default-block uniforms, coming from gl_spirv */
+ mode = vtn_variable_mode_uniform;
+ nir_mode = nir_var_uniform;
   }
   break;
case SpvStorageClassStorageBuffer:
@@ -1619,15 +1624,13 @@ vtn_storage_class_to_mode(struct vtn_builder *b,
   nir_mode = 0;
   break;
case SpvStorageClassUniformConstant:
-  if (glsl_type_is_image(interface_type->type)) {
+  if (glsl_type_is_image(interface_type->type))
  mode = vtn_variable_mode_image;
- nir_mode = nir_var_uniform;
-  } else if (glsl_type_is_sampler(interface_type->type)) {
+  else if (glsl_type_is_sampler(interface_type->type))
  mode = vtn_variable_mode_sampler;
- nir_mode = nir_var_uniform;
-  } else {
- vtn_fail("Invalid uniform constant variable type");
-  }
+  else
+ mode = vtn_variable_mode_uniform;
+  nir_mode = nir_var_uniform;
   break;
case SpvStorageClassPushConstant:
   mode = vtn_variable_mode_push_constant;
@@ -1795,11 +1798,13 @@ vtn_create_variable(struct vtn_builder *b, struct 
vtn_value *val,
case vtn_variable_mode_global:
case vtn_variable_mode_image:
case vtn_variable_mode_sampler:
+   case vtn_variable_mode_uniform:
   /* For these, we create the variable normally */
   var->var = rzalloc(b->shader, nir_variable);
   var->var->name = ralloc_strdup(var->var, val->name);
   var->var->type = var->type->type;
   var->var->data.mode = nir_mode;
+  var->var->data.location = -1;
 
   switch (var->mode) {
   case vtn_variable_mode_image:
-- 
2.14.1

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