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

[Francisco Jerez]

Jason Ekstrand <jason at jlekstrand.net> writes:

> On Dec 10, 2015 9:06 AM, "Francisco Jerez" <currojerez at riseup.net> wrote:
>>
>> Michael Schellenberger Costa <mschellenbergercosta at googlemail.com>
>> writes:
>>
>> > 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
>>
>> This is why the language provides a type-safe way to initialize a
>> variable.  Empty aggregate initializer please?
>
> What's the syntax for that in C++?  Does this work?
>
> book needs_pull[uniforms] = { false };
>

Yes, barring typos, or you can leave the aggregate initializer empty,
like '{}'.

>> >
>> >> +
>> >>     /* 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++;
>> >>        }
>> >>     }
>> >>
>> > _______________________________________________
>> > 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/20151211/29f262b0/attachment.sig>