[Mesa-dev] [PATCH] mesa: use u_bit_scan() in update_program_texture_state()

[Samuel Pitoiset]

On 05/10/2017 12:30 PM, Timothy Arceri wrote:
> On 10/05/17 20:06, Samuel Pitoiset wrote:
>> On 05/10/2017 03:01 AM, Timothy Arceri wrote:
>>> I made this same change a while back but found I couldn't really 
>>> measure any performance increase in practice. I suspect it's because 
>>> we already
>>> limit the max loops with (1 << s) <= prog[i]->SamplersUsed and there 
>>> is generally no big gaps in used samples.
>>>
>>> Did you manage to measure any improvement here?
>>
>> No, I didn't but I don't think it will improve anything. Though it's a 
>> common pratice to use a bit scan for bitfields, and it makes the code 
>> a bit cleaner.
> 
> It does have potential to provide a small improvement as this is a 
> hotpath, hence the opt you are replacing. Anyway looks good so feel free 
> to add:

Well, yeah maybe a small improvement. :)

> 
> Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
> 
>>
>>>
>>> On 10/05/17 07:33, Samuel Pitoiset wrote:
>>>> The check in update_single_program_texture() can also be
>>>> removed.
>>>>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>   src/mesa/main/texstate.c | 13 +++++--------
>>>>   1 file changed, 5 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
>>>> index 830b230b5d..c11a2e95fe 100644
>>>> --- a/src/mesa/main/texstate.c
>>>> +++ b/src/mesa/main/texstate.c
>>>> @@ -620,9 +620,6 @@ update_single_program_texture(struct gl_context 
>>>> *ctx, struct gl_program *prog,
>>>>      struct gl_sampler_object *sampler;
>>>>      int unit;
>>>> -   if (!(prog->SamplersUsed & (1 << s)))
>>>> -      return NULL;
>>>> -
>>>>      unit = prog->SamplerUnits[s];
>>>>      texUnit = &ctx->Texture.Unit[unit];
>>>> @@ -676,16 +673,16 @@ update_program_texture_state(struct gl_context 
>>>> *ctx, struct gl_program **prog,
>>>>      int i;
>>>>      for (i = 0; i < MESA_SHADER_STAGES; i++) {
>>>> +      GLbitfield mask;
>>>>         int s;
>>>>         if (!prog[i])
>>>>            continue;
>>>> -      /* We can't only do the shifting trick as the loop condition 
>>>> because if
>>>> -       * sampler 31 is active, the next iteration tries to shift by 
>>>> 32, which is
>>>> -       * undefined.
>>>> -       */
>>>> -      for (s = 0; s < MAX_SAMPLERS && (1 << s) <= 
>>>> prog[i]->SamplersUsed; s++) {
>>>> +      mask = prog[i]->SamplersUsed;
>>>> +
>>>> +      while (mask) {
>>>> +         const int s = u_bit_scan(&mask);
>>>>            struct gl_texture_object *texObj;
>>>>            texObj = update_single_program_texture(ctx, prog[i], s);
>>>>