[03/18] nir/linker: use empty block info to assign uniform locations

Submitted by apinheiro on June 29, 2018, 2:28 p.m.

Details

Message ID 20180629142904.15477-5-apinheiro@igalia.com
State New
Headers show

Not browsing as part of any series.

Patch hide | download patch | download mbox

diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c b/src/compiler/glsl/gl_nir_link_uniforms.c
index 388c1ab63fc..77d3eaa5f2b 100644
--- a/src/compiler/glsl/gl_nir_link_uniforms.c
+++ b/src/compiler/glsl/gl_nir_link_uniforms.c
@@ -79,6 +79,8 @@  nir_setup_uniform_remap_tables(struct gl_context *ctx,
    }
 
    /* Reserve locations for rest of the uniforms. */
+   link_util_update_empty_uniform_locations(prog);
+
    for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
       struct gl_uniform_storage *uniform = &prog->data->UniformStorage[i];
 
@@ -93,22 +95,23 @@  nir_setup_uniform_remap_tables(struct gl_context *ctx,
       if (uniform->remap_location != UNMAPPED_UNIFORM_LOC)
          continue;
 
-      /* How many new entries for this uniform? */
+      /* How many entries for this uniform? */
       const unsigned entries = MAX2(1, uniform->array_elements);
 
-      /* @FIXME: By now, we add un-assigned uniform locations to the end of
-       * the uniform file. We need to keep track of empty locations and use
-       * them.
-       */
-      unsigned chosen_location = prog->NumUniformRemapTable;
-
-      /* resize remap table to fit new entries */
-      prog->UniformRemapTable =
-         reralloc(prog,
-                  prog->UniformRemapTable,
-                  struct gl_uniform_storage *,
-                  prog->NumUniformRemapTable + entries);
-      prog->NumUniformRemapTable += entries;
+      unsigned chosen_location =
+         link_util_find_empty_block(prog, &prog->data->UniformStorage[i]);
+
+      if (chosen_location == -1) {
+         chosen_location = prog->NumUniformRemapTable;
+
+         /* resize remap table to fit new entries */
+         prog->UniformRemapTable =
+            reralloc(prog,
+                     prog->UniformRemapTable,
+                     struct gl_uniform_storage *,
+                     prog->NumUniformRemapTable + entries);
+         prog->NumUniformRemapTable += entries;
+      }
 
       /* set the base location in remap table for the uniform */
       uniform->remap_location = chosen_location;

Comments

On 30/06/18 00:28, Alejandro PiƱeiro wrote:
> For the cases of uniforms that doesn't have an explicit
> location. Under ARB_gl_spirv those are exceptions, like uniform atomic
> counters.
> ---
>   src/compiler/glsl/gl_nir_link_uniforms.c | 31 +++++++++++++++++--------------
>   1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c b/src/compiler/glsl/gl_nir_link_uniforms.c
> index 388c1ab63fc..77d3eaa5f2b 100644
> --- a/src/compiler/glsl/gl_nir_link_uniforms.c
> +++ b/src/compiler/glsl/gl_nir_link_uniforms.c
> @@ -79,6 +79,8 @@ nir_setup_uniform_remap_tables(struct gl_context *ctx,
>      }
>   
>      /* Reserve locations for rest of the uniforms. */
> +   link_util_update_empty_uniform_locations(prog);
> +
>      for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
>         struct gl_uniform_storage *uniform = &prog->data->UniformStorage[i];
>   
> @@ -93,22 +95,23 @@ nir_setup_uniform_remap_tables(struct gl_context *ctx,
>         if (uniform->remap_location != UNMAPPED_UNIFORM_LOC)
>            continue;
>   
> -      /* How many new entries for this uniform? */
> +      /* How many entries for this uniform? */
>         const unsigned entries = MAX2(1, uniform->array_elements);
>   
> -      /* @FIXME: By now, we add un-assigned uniform locations to the end of
> -       * the uniform file. We need to keep track of empty locations and use
> -       * them.
> -       */
> -      unsigned chosen_location = prog->NumUniformRemapTable;
> -
> -      /* resize remap table to fit new entries */
> -      prog->UniformRemapTable =
> -         reralloc(prog,
> -                  prog->UniformRemapTable,
> -                  struct gl_uniform_storage *,
> -                  prog->NumUniformRemapTable + entries);
> -      prog->NumUniformRemapTable += entries;
> +      unsigned chosen_location =

Please move  patch 2 to patch 1 and squash with this one with the 
current patch 1. This is just code churn.

As with my review of patch 1 please rename chosen_location -> location

Otherwise:

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


> +         link_util_find_empty_block(prog, &prog->data->UniformStorage[i]);
> +
> +      if (chosen_location == -1) {
> +         chosen_location = prog->NumUniformRemapTable;
> +
> +         /* resize remap table to fit new entries */
> +         prog->UniformRemapTable =
> +            reralloc(prog,
> +                     prog->UniformRemapTable,
> +                     struct gl_uniform_storage *,
> +                     prog->NumUniformRemapTable + entries);
> +         prog->NumUniformRemapTable += entries;
> +      }
>   
>         /* set the base location in remap table for the uniform */
>         uniform->remap_location = chosen_location;
>