[Mesa-dev] [PATCH v2 16/21] nir/linker: Add nir_build_program_resource_list()

[Timothy Arceri]

On 12/05/18 19:40, Alejandro Piñeiro wrote:
> From: Eduardo Lima Mitev <elima at igalia.com>
> 
> This function is equivalent to the linker.cpp
> build_program_resource_list() but will extract the resources from NIR
> shaders instead.
> 
> For now, only uniforms and program inputs are implemented.
> 
> v2: move from compiler/nir to compiler/glsl (Timothy Arceri)
> 
> Signed-off-by: Eduardo Lima <elima at igalia.com>
> Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com>
> ---
>   src/compiler/Makefile.sources     |  1 +
>   src/compiler/glsl/gl_nir_linker.c | 93 +++++++++++++++++++++++++++++++++++++++
>   src/compiler/glsl/gl_nir_linker.h |  3 ++
>   src/compiler/glsl/meson.build     |  1 +
>   4 files changed, 98 insertions(+)
>   create mode 100644 src/compiler/glsl/gl_nir_linker.c
> 
> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
> index 8104dd32002..409252ee587 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -30,6 +30,7 @@ LIBGLSL_FILES = \
>   	glsl/gl_nir_lower_samplers_as_deref.c \
>   	glsl/gl_nir_link_uniform_initializers.c \
>   	glsl/gl_nir_link_uniforms.c \
> +	glsl/gl_nir_linker.c \
>   	glsl/gl_nir_linker.h \
>   	glsl/gl_nir.h \
>   	glsl/glsl_parser_extras.cpp \
> diff --git a/src/compiler/glsl/gl_nir_linker.c b/src/compiler/glsl/gl_nir_linker.c
> new file mode 100644
> index 00000000000..155d94ef966
> --- /dev/null
> +++ b/src/compiler/glsl/gl_nir_linker.c
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "nir.h"
> +#include "gl_nir_linker.h"
> +#include "linker_util.h"
> +#include "main/mtypes.h"
> +#include "ir_uniform.h" /* for gl_uniform_storage */
> +
> +/* This file included general link methods, using NIR, instead of IR as
> + * the counter-part glsl/linker.cpp
> + *
> + * Also note that this is tailored for ARB_gl_spirv needs and particularities
> + */
> +
> +void
> +nir_build_program_resource_list(struct gl_context *ctx,
> +                                struct gl_shader_program *prog)
> +{
> +   /* Rebuild resource list. */
> +   if (prog->data->ProgramResourceList) {
> +      ralloc_free(prog->data->ProgramResourceList);
> +      prog->data->ProgramResourceList = NULL;
> +      prog->data->NumProgramResourceList = 0;
> +   }
> +
> +   struct set *resource_set = _mesa_set_create(NULL,
> +                                               _mesa_hash_pointer,
> +                                               _mesa_key_pointer_equal);
> +
> +   /* Add uniforms
> +    *
> +    * Here, it is expected that nir_link_uniforms() has already been
> +    * called, so that UniformStorage table is already available.
> +    */
> +   for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
> +      struct gl_uniform_storage *uniform = &prog->data->UniformStorage[i];
> +
> +      if (!link_util_add_program_resource(prog, resource_set, GL_UNIFORM, uniform,
> +                                          uniform->active_shader_mask)) {
> +         return;
> +      }
> +   }
> +
> +   /* Add inputs */
> +   struct gl_linked_shader *sh = prog->_LinkedShaders[MESA_SHADER_VERTEX];
> +   if (sh) {
> +      nir_shader *nir = sh->Program->nir;
> +      assert(nir);
> +
> +      nir_foreach_variable(var, &nir->inputs) {
> +         struct gl_shader_variable *sh_var =
> +            rzalloc(prog, struct gl_shader_variable);
> +
> +         /* ARB_gl_spirv: names are considered optional debug info, so the linker
> +          * needs to work without them, and returning them is optional. For
> +          * simplicity we ignore names.
> +          */
> +         sh_var->name = NULL;
> +         sh_var->type = var->type;
> +         sh_var->location = var->data.location;

This doesn't look right to me. Don't we need to handle structs, ifcs and 
arrays just like in the glsl code?


> +
> +         /* @TODO: Fill in the rest of gl_shader_variable data. */
> +
> +         if (!link_util_add_program_resource(prog, resource_set, GL_PROGRAM_INPUT,
> +                                             sh_var, 1 << MESA_SHADER_VERTEX)) {

Why 1 << MESA_SHADER_VERTEX? Does this spec not support using spriv with 
SSO?

> +            return;
> +         }
> +      }
> +   }
> +
> +   _mesa_set_destroy(resource_set, NULL);
> +}
> diff --git a/src/compiler/glsl/gl_nir_linker.h b/src/compiler/glsl/gl_nir_linker.h
> index 5c650ce0455..9567b9e7b8e 100644
> --- a/src/compiler/glsl/gl_nir_linker.h
> +++ b/src/compiler/glsl/gl_nir_linker.h
> @@ -37,6 +37,9 @@ bool gl_nir_link_uniforms(struct gl_context *ctx,
>   void gl_nir_set_uniform_initializers(struct gl_context *ctx,
>                                        struct gl_shader_program *prog);
>   
> +void nir_build_program_resource_list(struct gl_context *ctx,
> +                                     struct gl_shader_program *prog);
> +
>   #ifdef __cplusplus
>   } /* extern "C" */
>   #endif
> diff --git a/src/compiler/glsl/meson.build b/src/compiler/glsl/meson.build
> index 88a49c6997e..81d0fbea521 100644
> --- a/src/compiler/glsl/meson.build
> +++ b/src/compiler/glsl/meson.build
> @@ -71,6 +71,7 @@ files_libglsl = files(
>     'gl_nir_lower_samplers_as_deref.c',
>     'gl_nir_link_uniform_initializers.c',
>     'gl_nir_link_uniforms.c',
> +  'gl_nir_linker.c',
>     'gl_nir_linker.h',
>     'gl_nir.h',
>     'glsl_parser_extras.cpp',
>