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

2015-12-11 Thread Francisco Jerez
Jason Ekstrand  writes:

> On Dec 10, 2015 9:06 AM, "Francisco Jerez"  wrote:
>>
>> Michael Schellenberger Costa 
>> 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 */
>> >>   p

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

2015-12-10 Thread Jason Ekstrand
On Thu, Dec 10, 2015 at 1:12 AM, Michael Schellenberger Costa
 wrote:
> 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

Yes, that's what we want.  I made and fixed this mistake several times
while working on another patch, but I guess it leaked into this one.
I'll get that fixed.

>> +
>> /* 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2015-12-10 Thread Jason Ekstrand
On Dec 10, 2015 9:06 AM, "Francisco Jerez"  wrote:
>
> Michael Schellenberger Costa 
> 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 };

> >
> >> +
> >> /* 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@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___

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

2015-12-10 Thread Francisco Jerez
Michael Schellenberger Costa 
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?

>
>> +
>> /* 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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2015-12-10 Thread 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++;
>}
> }
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2015-12-09 Thread 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));
+
/* 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++;
   }
}
-- 
2.5.0.400.gff86faf

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev