[Mesa-dev] [PATCH v2 19/21] i965: account for NIR uniforms without name

[Timothy Arceri]

Look like we should have just always done it this way.

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

On 12/05/18 19:40, Alejandro Piñeiro wrote:
> From: Eduardo Lima Mitev <elima at igalia.com>
> 
> Right now, the BRW linker code assumes nir_variable::name is always
> non-NULL, but thanks to ARB_gl_spirv we will soon be linking SPIR-V
> programs, and those explicitly require matching uniforms by location.
> The name is just a debug hint.
> 
> Instead of checking for the name this patch makes it check for
> var->num_state_slots on the assumption that everything that had an
> internal name also had some state slots. This seems likely because the
> two code paths that are taken when the name begins with "gl_" already
> have an assert that var->state_slots is not NULL.
> 
> v2: simplified, most of it moved to glsl/nir/spirv (Neil Roberts)
> v3: check for num_state_slots instead of the name. This is needed
>      because we do actually have nameless builtins with SPIR-V such as
>      PatchVerticesIn and we want them to hit the
>      _mesa_add_state_reference code path (Neil Roberts)
> 
> Signed-off-by: Eduardo Lima <elima at igalia.com>
> Signed-off-by: Neil Roberts <nroberts at igalia.com>
> ---
>   src/mesa/drivers/dri/i965/brw_link.cpp         | 11 ++++-------
>   src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp |  2 +-
>   2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
> index 0203c44f1cb..c75721c06ca 100644
> --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> @@ -321,13 +321,10 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
>          * get sent to the shader.
>          */
>         nir_foreach_variable(var, &prog->nir->uniforms) {
> -         if (strncmp(var->name, "gl_", 3) == 0) {
> -            const nir_state_slot *const slots = var->state_slots;
> -            assert(var->state_slots != NULL);
> -
> -            for (unsigned int i = 0; i < var->num_state_slots; i++) {
> -               _mesa_add_state_reference(prog->Parameters, slots[i].tokens);
> -            }
> +         const nir_state_slot *const slots = var->state_slots;
> +         for (unsigned int i = 0; i < var->num_state_slots; i++) {
> +            assert(slots != NULL);
> +            _mesa_add_state_reference(prog->Parameters, slots[i].tokens);
>            }
>         }
>      }
> diff --git a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> index 69da83ad364..64c745b609c 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> @@ -202,7 +202,7 @@ brw_nir_setup_glsl_uniforms(void *mem_ctx, nir_shader *shader,
>         if (var->interface_type != NULL || var->type->contains_atomic())
>            continue;
>   
> -      if (strncmp(var->name, "gl_", 3) == 0) {
> +      if (var->num_state_slots > 0) {
>            brw_nir_setup_glsl_builtin_uniform(var, prog, stage_prog_data,
>                                               is_scalar);
>         } else {
>