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

[Jason Ekstrand]

On Fri, Feb 24, 2017 at 8:33 AM, tournier.elie <tournier.elie at gmail.com>
wrote:

> Ping.
>
>
> On 13 February 2017 at 10:17, tournier.elie <tournier.elie at gmail.com>
> wrote:
>
>> Whoops I answer 2 times.
>>
>> Would the comment bellow be appropriate?
>>
>> This limit is chosen fairly arbitrarily.
>> GLSL IR max iteration is 32 instructions. (Multiply counting nodes and
>> magic number 5).
>> But there is no 1:1 mapping between GLSL IR and NIR so 25 was picked
>> because
>> it seemed to give about the same results. Around 5 instructions per node.
>> But some loops that would unroll with GLSL IR fail to unroll if we set
>> this to 25 so we set it to 26.
>>
>
Sorry its taken so long.  That comment looks good to me.

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

I'll push it as soon as I've verified that it builds.

--Jason


> Elie
>>
>>
>> On 13 February 2017 at 01:00, tournier.elie <tournier.elie at gmail.com>
>> wrote:
>>
>>>
>>> On 9 February 2017 at 02:48, Timothy Arceri <tarceri at itsqueeze.com>
>>> wrote:
>>>
>>> > On Wed, 8 Feb 2017 15:54:46 -0800
>>> > Jason Ekstrand <jason at jlekstrand.net> wrote:
>>> >
>>> > > On Wed, Feb 8, 2017 at 2:20 PM, Elie Tournier
>>> > > <tournier.elie at gmail.com> wrote:
>>> > >
>>> > > > Signed-off-by: Elie Tournier <tournier.elie at gmail.com>
>>> > > > ---
>>> > > >  src/compiler/nir/nir_opt_loop_unroll.c | 10 +++++++++-
>>> > > >  1 file changed, 9 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..035a030239 100644
>>> > > > --- a/src/compiler/nir/nir_opt_loop_unroll.c
>>> > > > +++ b/src/compiler/nir/nir_opt_loop_unroll.c
>>> > > > @@ -26,6 +26,14 @@
>>> > > >  #include "nir_control_flow.h"
>>> > > >  #include "nir_loop_analyze.h"
>>> > > >
>>> > > > +
>>> > > > +/* 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.
>>> > > >
>>> > >
>>> > > Ok, I lied in my comment.  It's not 25, it's 32.  Tim, can you
>>> > > explain the discrepancy?
>>> > > --Jason
>>> >
>>> > Max iteration is 32 this is number of instructions, in GLSL IR it was
>>> > counting nodes and the magic number was 5. There is no 1:1 mapping 25
>>> > was picked because it seemed to give about the same results. Around 5
>>> > instructions per node.
>>> >
>>> >
>>> So to resume. Is this comment correct for you?
>>>
>>> This limit is chosen fairly arbitrarily.
>>> GLSL IR max iteration is 32 instructions. (Counting nodes by magic
>>> number 5).
>>> But there is no 1:1 mapping between GLSL IR and NIR so 25 was picked
>>> because
>>> it seemed to give about the same results. Around 5 instructions per node.
>>> But some loops that would unroll with GLSL IR fail to unroll if we set
>>> this to 25 so we set it to 26.
>>>
>>> >
>>> > >
>>> > > > + */
>>> > > > +#define LOOP_UNROLL_LIMIT 26
>>> > > > +
>>> > > >  /* 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 +468,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 *
>>> > > > LOOP_UNROLL_LIMIT;
>>> > > >
>>> > > >     return loop_not_too_large;
>>> > > >  }
>>> > > > --
>>> > > > 2.11.0
>>> > > >
>>> > > >
>>> >
>>> >
>>>
>>>
>>> On 9 February 2017 at 02:48, Timothy Arceri <tarceri at itsqueeze.com>
>>> wrote:
>>>
>>>> On Wed, 8 Feb 2017 15:54:46 -0800
>>>> Jason Ekstrand <jason at jlekstrand.net> wrote:
>>>>
>>>> > On Wed, Feb 8, 2017 at 2:20 PM, Elie Tournier
>>>> > <tournier.elie at gmail.com> wrote:
>>>> >
>>>> > > Signed-off-by: Elie Tournier <tournier.elie at gmail.com>
>>>> > > ---
>>>> > >  src/compiler/nir/nir_opt_loop_unroll.c | 10 +++++++++-
>>>> > >  1 file changed, 9 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..035a030239 100644
>>>> > > --- a/src/compiler/nir/nir_opt_loop_unroll.c
>>>> > > +++ b/src/compiler/nir/nir_opt_loop_unroll.c
>>>> > > @@ -26,6 +26,14 @@
>>>> > >  #include "nir_control_flow.h"
>>>> > >  #include "nir_loop_analyze.h"
>>>> > >
>>>> > > +
>>>> > > +/* 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.
>>>> > >
>>>> >
>>>> > Ok, I lied in my comment.  It's not 25, it's 32.  Tim, can you
>>>> > explain the discrepancy?
>>>> > --Jason
>>>>
>>>>
>>> Max iteration is 32 this is number of instructions, in GLSL IR it was
>>>> counting nodes and the magic number was 5. There is no 1:1 mapping 25
>>>> was picked because it seemed to give about the same results. Around 5
>>>> instructions per node.
>>>>
>>>>
>>> So to resume. Is this comment correct for you?
>>>
>>> Max iteration is 32 this is number of instructions, in GLSL IR it was
>>> counting nodes and the magic number was 5. There is no 1:1 mapping 25
>>> was picked because it seemed to give about the same results. Around 5
>>> instructions per node. But some loops that would unroll with GLSL IR fail
>>> to unroll if we set this to 25 so we set it to 26.
>>>
>>> >
>>>> >
>>>> > > + */
>>>> > > +#define LOOP_UNROLL_LIMIT 26
>>>> > > +
>>>> > >  /* 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 +468,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 *
>>>> > > LOOP_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/20170224/d9e8855f/attachment-0001.html>