[Mesa-dev] [PATCH 11/20] i965/fs: Append uniform entries to the gather table

[Francisco Jerez]

Abdiel Janulgue <abdiel.janulgue at linux.intel.com> writes:

> This patch generates the gather table entries for ordinary uniforms
> if they are present. The uniform constants here will later be packed
> together with UBO constants.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index d240371..e39d821 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1863,8 +1863,11 @@ fs_visitor::assign_constant_locations()
>        }
>     }
>  
> -   stage_prog_data->nr_params = num_push_constants;
>     stage_prog_data->nr_pull_params = num_pull_constants;
> +   stage_prog_data->nr_params = 0;

I doubt it's useful to set this to zero here only to calculate it again
below.

> +
> +   unsigned const_reg_access[uniforms];

VLAs are a non-standard extension to C++.  Usually I'd recommend to use
the C++ standard library instead, but because it's banned from the i965
back-end I guess the best you can get is new[]/delete[].

> +   memset(const_reg_access, 0, sizeof(const_reg_access));
>
This memset would be unnecessary if you used new[] to value-initialize
the array.

>     /* Up until now, the param[] array has been indexed by reg + reg_offset
>      * of UNIFORM registers.  Move pull constants into pull_param[] and
> @@ -1881,8 +1884,21 @@ fs_visitor::assign_constant_locations()
>           stage_prog_data->pull_param[pull_constant_loc[i]] = value;
>        } else if (push_constant_loc[i] != -1) {
>           stage_prog_data->param[push_constant_loc[i]] = value;
> +         int p = stage_prog_data->nr_params++;
> +
> +         /* access table for uniform registers*/
> +         const_reg_access[(ALIGN(prog_data->nr_params, 4) / 4) - 1] |=
> +            (1 << (p % 4));

Actually why do you need the const_reg_access array?  You could assign
the channel mask straight away like:

|  const unsigned p = push_constant_loc[i];
|  stage_prog_data->gather_table[p / 4].channel_mask |= 1 << (p % 4);

Then drop the loop below.

Is there any reason why you leave gather table entries half-initialized
at this point?  It would make sense to initialize const_offset to
ALIGN(4 * p, 16) already so you can handle UBO and non-UBO entries
consistently later on in gen7_submit_gather_table().

>        }
>     }
> +
> +   int num_consts = ALIGN(prog_data->nr_params, 4) / 4;
> +   for (int i = 0; i < num_consts; i++) {
> +      int p = stage_prog_data->nr_gather_table++;
> +      stage_prog_data->gather_table[p].reg = -1;
> +      stage_prog_data->gather_table[p].channel_mask =
> +         const_reg_access[i];
> +   }
>  }
>  
>  /**
> -- 
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151023/c06c586d/attachment.sig>