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

[tournier.elie]

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.

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/20170213/ab9e5f30/attachment-0001.html>