[Mesa-dev] [PATCH 09/22] spirv: Get rid of vtn_variable_mode_image/sampler
Alejandro Piñeiro
apinheiro at igalia.com
Tue Apr 17 14:42:45 UTC 2018
I forgot to CC any of the radv people. Sorry for the noise.
On 17/04/18 16:36, Alejandro Piñeiro wrote:
> From: Neil Roberts <nroberts at igalia.com>
>
> vtn_variable_mode_image and _sampler are instead replaced with
> vtn_variable_mode_uniform which encompasses both of them. In the few
> places where it was neccessary to distinguish between the two, the
> GLSL type of the pointer is used instead.
>
> The main reason to do this is that on OpenGL it is permitted to put
> images and samplers into structs and declare a uniform with them. That
> means that variables can now have a mix of uniform, sampler and image
> modes so picking a single one of those modes for a variable no longer
> makes sense.
>
> This fixes OpLoad on a sampler within a struct which was previously
> using the variable mode to determine whether it was a sampler or not.
> The type of the variable is a struct so it was not being considered to
> be uniform mode even though the member being loaded should be sampler
> mode.
>
> Signed-off-by: Eduardo Lima <elima at igalia.com>
> Signed-off-by: Neil Roberts <nroberts at igalia.com
> Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com>
> ---
>
> Warning to radv people: this commit is a non-trivial change of how
> image/samplers are managed. Specifically, could lead to the interface
> type of image/sampler nir_variable to be NULL. We needed to fix this
> on anv on the following commit "anv/nir: Use nir_variable's type if
> interface_type is null" to avoid some CTS tests to crash. Probably
> radv would need to do something similar.
>
>
> src/compiler/spirv/spirv_to_nir.c | 4 ++--
> src/compiler/spirv/vtn_cfg.c | 4 ++--
> src/compiler/spirv/vtn_private.h | 2 --
> src/compiler/spirv/vtn_variables.c | 43 +++++++++++---------------------------
> 4 files changed, 16 insertions(+), 37 deletions(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c
> index 28274311c2b..b427205affe 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -3710,10 +3710,10 @@ vtn_handle_body_instruction(struct vtn_builder *b, SpvOp opcode,
> case SpvOpImageQuerySize: {
> struct vtn_pointer *image =
> vtn_value(b, w[3], vtn_value_type_pointer)->pointer;
> - if (image->mode == vtn_variable_mode_image) {
> + if (glsl_type_is_image(image->type->type)) {
> vtn_handle_image(b, opcode, w, count);
> } else {
> - vtn_assert(image->mode == vtn_variable_mode_sampler);
> + vtn_assert(glsl_type_is_sampler(image->type->type));
> vtn_handle_texture(b, opcode, w, count);
> }
> break;
> diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
> index e7d2f9ea614..2c3bf698cc2 100644
> --- a/src/compiler/spirv/vtn_cfg.c
> +++ b/src/compiler/spirv/vtn_cfg.c
> @@ -124,10 +124,10 @@ vtn_cfg_handle_prepass_instruction(struct vtn_builder *b, SpvOp opcode,
> without_array = without_array->array_element;
>
> if (glsl_type_is_image(without_array->type)) {
> - vtn_var->mode = vtn_variable_mode_image;
> + vtn_var->mode = vtn_variable_mode_uniform;
> param->interface_type = without_array->type;
> } else if (glsl_type_is_sampler(without_array->type)) {
> - vtn_var->mode = vtn_variable_mode_sampler;
> + vtn_var->mode = vtn_variable_mode_uniform;
> param->interface_type = without_array->type;
> } else {
> vtn_var->mode = vtn_variable_mode_param;
> diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
> index 183024e14f4..98bec389fcd 100644
> --- a/src/compiler/spirv/vtn_private.h
> +++ b/src/compiler/spirv/vtn_private.h
> @@ -406,8 +406,6 @@ enum vtn_variable_mode {
> vtn_variable_mode_ubo,
> vtn_variable_mode_ssbo,
> vtn_variable_mode_push_constant,
> - vtn_variable_mode_image,
> - vtn_variable_mode_sampler,
> vtn_variable_mode_workgroup,
> vtn_variable_mode_input,
> vtn_variable_mode_output,
> diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c
> index 0996d37e930..633fe6bd9da 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1541,9 +1541,7 @@ 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 if (vtn_var->mode != vtn_variable_mode_uniform &&
> - vtn_var->mode != vtn_variable_mode_sampler &&
> - vtn_var->mode != vtn_variable_mode_image) {
> + } else if (vtn_var->mode != vtn_variable_mode_uniform) {
> vtn_warn("Location must be on input, output, uniform, sampler or "
> "image variable");
> return;
> @@ -1621,12 +1619,7 @@ vtn_storage_class_to_mode(struct vtn_builder *b,
> nir_mode = 0;
> break;
> case SpvStorageClassUniformConstant:
> - if (glsl_type_is_image(interface_type->type))
> - mode = vtn_variable_mode_image;
> - else if (glsl_type_is_sampler(interface_type->type))
> - mode = vtn_variable_mode_sampler;
> - else
> - mode = vtn_variable_mode_uniform;
> + mode = vtn_variable_mode_uniform;
> nir_mode = nir_var_uniform;
> break;
> case SpvStorageClassPushConstant:
> @@ -1769,11 +1762,11 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
> case vtn_variable_mode_ssbo:
> b->shader->info.num_ssbos++;
> break;
> - case vtn_variable_mode_image:
> - b->shader->info.num_images++;
> - break;
> - case vtn_variable_mode_sampler:
> - b->shader->info.num_textures++;
> + case vtn_variable_mode_uniform:
> + if (glsl_type_is_image(without_array->type))
> + b->shader->info.num_images++;
> + else if (glsl_type_is_sampler(without_array->type))
> + b->shader->info.num_textures++;
> break;
> case vtn_variable_mode_push_constant:
> b->shader->num_uniforms = vtn_type_block_size(b, type);
> @@ -1793,8 +1786,6 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
> switch (var->mode) {
> case vtn_variable_mode_local:
> 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);
> @@ -1802,16 +1793,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
> 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:
> - case vtn_variable_mode_sampler:
> - var->var->interface_type = without_array->type;
> - break;
> - default:
> - var->var->interface_type = NULL;
> - break;
> - }
> + var->var->interface_type = NULL;
> break;
>
> case vtn_variable_mode_workgroup:
> @@ -1936,8 +1918,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
>
> vtn_foreach_decoration(b, val, var_decoration_cb, var);
>
> - if (var->mode == vtn_variable_mode_image ||
> - var->mode == vtn_variable_mode_sampler) {
> + if (var->mode == vtn_variable_mode_uniform) {
> /* XXX: We still need the binding information in the nir_variable
> * for these. We should fix that.
> */
> @@ -1945,7 +1926,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,
> var->var->data.descriptor_set = var->descriptor_set;
> var->var->data.index = var->input_attachment_index;
>
> - if (var->mode == vtn_variable_mode_image)
> + if (glsl_type_is_image(without_array->type))
> var->var->data.image.format = without_array->image_format;
> }
>
> @@ -2085,8 +2066,8 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp opcode,
>
> vtn_assert_types_equal(b, opcode, res_type, src_val->type->deref);
>
> - if (src->mode == vtn_variable_mode_image ||
> - src->mode == vtn_variable_mode_sampler) {
> + if (glsl_type_is_image(res_type->type) ||
> + glsl_type_is_sampler(res_type->type)) {
> vtn_push_value(b, w[2], vtn_value_type_pointer)->pointer = src;
> return;
> }
More information about the mesa-dev
mailing list