[Mesa-dev] [PATCH] nir: delete magic number

[tournier.elie]

On 8 February 2017 at 17:12, Jason Ekstrand <jason at jlekstrand.net> wrote:

> On Wed, Feb 8, 2017 at 8:56 AM, Elie Tournier <tournier.elie at gmail.com>
> wrote:
>
>> Explaining what is the "26" for.
>>
>> PS: I don't have git rights. Can you push this patch for me?
>>
>> Signed-off-by: Elie Tournier <tournier.elie at gmail.com>
>> ---
>>  src/compiler/nir/nir_opt_loop_unroll.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/compiler/nir/nir_opt_loop_unroll.c
>> b/src/compiler/nir/nir_opt_loop_unroll.c
>> index 37cbced43d..2bb1a57791 100644
>> --- a/src/compiler/nir/nir_opt_loop_unroll.c
>> +++ b/src/compiler/nir/nir_opt_loop_unroll.c
>> @@ -26,6 +26,8 @@
>>  #include "nir_control_flow.h"
>>  #include "nir_loop_analyze.h"
>>
>> +#define GLSL_IR_UNROLL_LIMIT 26
>>
>
> Maybe just call this LOOP_UNROLL_LIMIT?  It's not exactly the same as the
> GLSL IR limit.  Also, a comment would be good.  How about:
>
> This limit is chosen fairly arbitrarily.  The GLSL IR limit is 25.
> However, due to slight differences in the way the two IRs count
> instructions, some loops that would unroll with GLSL IR fail to unroll if
> we set this to 25 so we set it to 26.
>

Thanks for the review. I will submit a v2.

>
>
>> +
>>  /* Prepare this loop for unrolling by first converting to lcssa and then
>>   * converting the phis from the loops first block and the block that
>> follows
>>   * the loop into regs.  Partially converting out of SSA allows us to
>> unroll
>> @@ -460,7 +462,7 @@ is_loop_small_enough_to_unroll(nir_shader *shader,
>> nir_loop_info *li)
>>        return true;
>>
>>     bool loop_not_too_large =
>> -      li->num_instructions * li->trip_count <= max_iter * 26;
>> +      li->num_instructions * li->trip_count <= max_iter *
>> GLSL_IR_UNROLL_LIMIT;
>>
>>     return loop_not_too_large;
>>  }
>> --
>> 2.11.0
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170208/a3a0744c/attachment.html>