[Mesa-dev] [PATCH 05/15] i965/fs: Stop relying on param_size in assign_constant_locations

[Michael Schellenberger Costa]

Hi,

Am 10.12.2015 um 05:23 schrieb Jason Ekstrand:
> Now that we have MOV_INDIRECT opcodes, we have all of the size information
> we need directly in the opcode.  With a little restructuring of the
> algorithm used in assign_constant_locations we don't need param_size
> anymore.  The big thing to watch out for now, however, is that you can have
> two ranges overlap where neither contains the other.  In order to deal with
> this, we make the first pass just flag what needs pulling and handle
> assigning pull constant locations until later.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 44 ++++++++++++++----------------------
>  1 file changed, 17 insertions(+), 27 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 786c5fb..1add656 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1920,14 +1920,12 @@ fs_visitor::assign_constant_locations()
>     if (dispatch_width != 8)
>        return;
>  
> -   unsigned int num_pull_constants = 0;
> -
> -   pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
> -   memset(pull_constant_loc, -1, sizeof(pull_constant_loc[0]) * uniforms);
> -
>     bool is_live[uniforms];
>     memset(is_live, 0, sizeof(is_live));
>  
> +   bool needs_pull[uniforms];
> +   memset(needs_pull, 0, sizeof(is_live));
While it is valid, could you make this sizeof(needs_pull) to be safe?
Michael

> +
>     /* First, we walk through the instructions and do two things:
>      *
>      *  1) Figure out which uniforms are live.
> @@ -1943,20 +1941,15 @@ fs_visitor::assign_constant_locations()
>           if (inst->src[i].file != UNIFORM)
>              continue;
>  
> -         if (inst->opcode == SHADER_OPCODE_MOV_INDIRECT && i == 0) {
> -            int uniform = inst->src[0].nr;
> +         int constant_nr = inst->src[i].nr + inst->src[i].reg_offset;
>  
> -            /* If this array isn't already present in the pull constant buffer,
> -             * add it.
> -             */
> -            if (pull_constant_loc[uniform] == -1) {
> -               assert(param_size[uniform]);
> -               for (int j = 0; j < param_size[uniform]; j++)
> -                  pull_constant_loc[uniform + j] = num_pull_constants++;
> +         if (inst->opcode == SHADER_OPCODE_MOV_INDIRECT && i == 0) {
> +            for (unsigned j = 0; j < inst->src[2].ud / 4; j++) {
> +               is_live[constant_nr + j] = true;
> +               needs_pull[constant_nr + j] = true;
>              }
>           } else {
>              /* Mark the the one accessed uniform as live */
> -            int constant_nr = inst->src[i].nr + inst->src[i].reg_offset;
>              if (constant_nr >= 0 && constant_nr < (int) uniforms)
>                 is_live[constant_nr] = true;
>           }
> @@ -1973,26 +1966,23 @@ fs_visitor::assign_constant_locations()
>      */
>     unsigned int max_push_components = 16 * 8;
>     unsigned int num_push_constants = 0;
> +   unsigned int num_pull_constants = 0;
>  
>     push_constant_loc = ralloc_array(mem_ctx, int, uniforms);
> +   pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
>  
>     for (unsigned int i = 0; i < uniforms; i++) {
> -      if (!is_live[i] || pull_constant_loc[i] != -1) {
> -         /* This UNIFORM register is either dead, or has already been demoted
> -          * to a pull const.  Mark it as no longer living in the param[] array.
> -          */
> -         push_constant_loc[i] = -1;
> +      push_constant_loc[i] = -1;
> +      pull_constant_loc[i] = -1;
> +
> +      if (!is_live[i])
>           continue;
> -      }
>  
> -      if (num_push_constants < max_push_components) {
> -         /* Retain as a push constant.  Record the location in the params[]
> -          * array.
> -          */
> +      if (!needs_pull[i] && num_push_constants < max_push_components) {
> +         /* Retain as a push constant */
>           push_constant_loc[i] = num_push_constants++;
>        } else {
> -         /* Demote to a pull constant. */
> -         push_constant_loc[i] = -1;
> +         /* We have to pull it */
>           pull_constant_loc[i] = num_pull_constants++;
>        }
>     }
>