[Mesa-dev] [PATCH] i965/cs: Don't use a thread channel ID for small local sizes

[Ian Romanick]

On 10/19/2016 02:26 PM, Jordan Justen wrote:
> On 2016-10-19 13:47:35, Ian Romanick wrote:
>> On 10/19/2016 12:45 PM, Jordan Justen wrote:
>>> When the local group size is 8 or less, we will execute the program at
>>> most 1 time. Therefore, the local channel ID will always be 0. By
>>> using a constant 0 in this case we can prevent using push constant
>>> data.
>>>
>>> This is not expected to be common a occurance in real applications,
>>> but it has been seen in tests.
>>>
>>> We could extend this optimization to 16 and 32 for SIMD16 and SIMD32,
>>> but it gets a bit more complicated, because this optimization is
>>> currently being done early on, before we have decided the SIMD size.
>>>
>>> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_nir_intrinsics.c | 13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_nir_intrinsics.c b/src/mesa/drivers/dri/i965/brw_nir_intrinsics.c
>>> index 9ae161f..60f302e 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_nir_intrinsics.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_nir_intrinsics.c
>>> @@ -39,12 +39,21 @@ struct lower_intrinsics_state {
>>>  static nir_ssa_def *
>>>  read_thread_local_id(struct lower_intrinsics_state *state)
>>>  {
>>> +   nir_builder *b = &state->builder;
>>> +   nir_shader *nir = state->nir;
>>> +   const unsigned *sizes = nir->info.cs.local_size;
>>> +   const group_size = sizes[0] * sizes[1] * sizes[2];
>>            ^
>>             Isn't this missing a type?  Did you test this? :)
> 
> Yikes. Yeah, I sent it through jenkins. I guess it defaults to int in
> this case:
> 
>   CC       brw_nir_intrinsics.lo
> brw_nir_intrinsics.c: In function ‘read_thread_local_id’:
> brw_nir_intrinsics.c:45:10: warning: type defaults to ‘int’ in declaration of ‘group_size’ [-Wimplicit-int]
>     const group_size = sizes[0] * sizes[1] * sizes[2];
>           ^

Thanks C programming language.  You're a real hero. :(

> I'll add unsigned.

With that, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> -Jordan
> 
>>> +
>>> +   /* Some programs have local_size dimensions so small that the thread local
>>> +    * ID will always be 0.
>>> +    */
>>> +   if (group_size <= 8)
>>> +      return nir_imm_int(b, 0);
>>> +
>>>     assert(state->cs_prog_data->thread_local_id_index >= 0);
>>>     state->cs_thread_id_used = true;
>>>     const int id_index = state->cs_prog_data->thread_local_id_index;
>>>  
>>> -   nir_builder *b = &state->builder;
>>> -   nir_shader *nir = state->nir;
>>>     nir_intrinsic_instr *load =
>>>        nir_intrinsic_instr_create(nir, nir_intrinsic_load_uniform);
>>>     load->num_components = 1;
>>>