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

Timothy Arceri tarceri at itsqueeze.com
Thu May 17 06:56:21 UTC 2018



On 15/05/18 01:05, Alejandro Piñeiro wrote:
> On 14/05/18 01:26, Timothy Arceri wrote:
>> 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?
> 
> About those two comments: my bad. As mentioned this subseries is mostly
> about basic uniform support, and the minimal amount of extras needed to
> get uniform tests running. The code below "/* Add inputs*/ is a wip code
> for inputs that got slipped on that patch as part of our rebases/squashes.
> 
> So the final patch will not have that part.
> 
> Sorry for the noise.

Ok if you remove all these bits that this patch is:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

> 
>>
>>> +            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',
>>>
>>
> 


More information about the mesa-dev mailing list