[Mesa-dev] [PATCH 4/4] i965/nir: Sort uniforms direct-first and use two different uniform registers

[Jason Ekstrand]

Forgot the shader-db numbers:

total instructions in shared programs: 4114840 -> 4112172 (-0.06%)
instructions in affected programs:     43316 -> 40648 (-6.16%)
helped:                                116
HURT:                                  0
GAINED:                                0
LOST:                                  0



On Wed, Mar 18, 2015 at 4:45 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> Previously, we put all the uniforms into one big array.  The problem with
> this approach is that, as soon as there was one indirect array acces, the
> backend would decide that the entire large array should be pull constants.
> This commit splits the array in half: first direct-only uniforms and then
> potentially-indirect uniforms.  This may not be optimal, but it does let
> the backend promote things to push constants.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.h       |  4 +++-
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 26 ++++++++++++++++++++------
>  2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index a520fd4..9f1d8fa 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -433,6 +433,9 @@ public:
>     /** Number of uniform variable components visited. */
>     unsigned uniforms;
>
> +   /** Total number of direct uniforms we can get from NIR */
> +   unsigned num_direct_uniforms;
> +
>     /** Byte-offset for the next available spot in the scratch space buffer. */
>     unsigned last_scratch;
>
> @@ -466,7 +469,6 @@ public:
>     fs_reg *nir_globals;
>     fs_reg nir_inputs;
>     fs_reg nir_outputs;
> -   fs_reg nir_uniforms;
>     fs_reg *nir_system_values;
>
>     /** @{ debug annotation info */
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index c316088..429f2bb 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -106,7 +106,9 @@ fs_visitor::emit_nir_code()
>     /* Get rid of split copies */
>     nir_optimize(nir);
>
> -   nir_assign_var_locations_packed(&nir->uniforms, &nir->num_uniforms);
> +   nir_assign_var_locations_packed_direct_first(nir, &nir->uniforms,
> +                                                &num_direct_uniforms,
> +                                                &nir->num_uniforms);
>     nir_assign_var_locations_packed(&nir->inputs, &nir->num_inputs);
>     nir_assign_var_locations_packed(&nir->outputs, &nir->num_outputs);
>
> @@ -177,7 +179,6 @@ fs_visitor::emit_nir_code()
>     }
>
>     if (nir->num_uniforms > 0) {
> -      nir_uniforms = fs_reg(UNIFORM, 0);
>        nir_setup_uniforms(nir);
>     }
>
> @@ -308,7 +309,12 @@ void
>  fs_visitor::nir_setup_uniforms(nir_shader *shader)
>  {
>     uniforms = shader->num_uniforms;
> -   param_size[0] = shader->num_uniforms;
> +
> +   /* We split the uniform register file in half.  The first half is
> +    * entirely direct uniforms.  The second half is indirect.
> +    */
> +   param_size[0] = num_direct_uniforms;
> +   param_size[num_direct_uniforms] = shader->num_uniforms - num_direct_uniforms;
>
>     if (dispatch_width != 8)
>        return;
> @@ -1484,11 +1490,19 @@ fs_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
>     case nir_intrinsic_load_uniform_indirect:
>        has_indirect = true;
>     case nir_intrinsic_load_uniform: {
> -      unsigned index = 0;
> +      unsigned index = instr->const_index[0];
> +
> +      fs_reg uniform_reg;
> +      if (index < num_direct_uniforms) {
> +         uniform_reg = fs_reg(UNIFORM, 0);
> +      } else {
> +         uniform_reg = fs_reg(UNIFORM, num_direct_uniforms);
> +         index -= num_direct_uniforms;
> +      }
> +
>        for (int i = 0; i < instr->const_index[1]; i++) {
>           for (unsigned j = 0; j < instr->num_components; j++) {
> -            fs_reg src = offset(retype(nir_uniforms, dest.type),
> -                                instr->const_index[0] + index);
> +            fs_reg src = offset(retype(uniform_reg, dest.type), index);
>              if (has_indirect)
>                 src.reladdr = new(mem_ctx) fs_reg(get_nir_src(instr->src[0]));
>              index++;
> --
> 2.3.2
>