Re: [Mesa-dev] [PATCH 03/10] nir: add helpers to check if we can unroll loops

[Timothy Arceri]

On Fri, 2016-09-16 at 16:52 +0200, Erik Faye-Lund wrote:
> On Thu, Sep 15, 2016 at 9:03 AM, Timothy Arceri
>  wrote:
> > 
> > This will be used by the loop unroll and lcssa passes.
> > 
> > V2:
> > - Check instruction count is not too large for unrolling
> > - Add helper for complex loop unrolling
> > ---
> >  src/compiler/nir/nir.h | 31 +++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > index 49e8cd8..3a2a13a 100644
> > --- a/src/compiler/nir/nir.h
> > +++ b/src/compiler/nir/nir.h
> > @@ -2590,6 +2590,37 @@ bool nir_normalize_cubemap_coords(nir_shader
> > *shader);
> > 
> >  void nir_live_ssa_defs_impl(nir_function_impl *impl);
> > 
> > +static inline bool
> > +is_loop_small_enough_to_unroll(nir_shader *shader, nir_loop_info
> > *li)
> > +{
> > +   unsigned max_iter = shader->options->max_unroll_iterations;
> > +
> > +   if (li->trip_count > max_iter)
> > +  return false;
> > +
> > +   if (li->force_unroll)
> > +  return true;
> > +
> > +   bool loop_not_too_large =
> > +  li->num_instructions * li->trip_count <= max_iter * 25;
> 
> 
> "max_iter * 25" seems like a pretty arbirary limit at first glance.
> How was it found? Perhaps a comment explaining a bit could be added?

Well it is :P I just tried to match it somewhat to the GLSL IR pass. I
don't think there was even a great explanation for the value that was
chosen.

> 
> > 
> > +static inline bool
> > +is_complex_loop(nir_shader *shader, nir_loop_info *li)
> > +{
> > +   unsigned num_lt = list_length(>loop_terminator_list);
> > +   return is_loop_small_enough_to_unroll(shader, li) && num_lt ==
> > 2;
> 
> Perhaps you could add a comment to explain the "num_lt == 2"-part?

Sure. Basically if we don't know the trip count of all the exits (not a
simple loop) we can only possibly unroll loops with two exit points.
Anything more would be extra code for a scenario that is unlikely to
come up very often.

> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 03/10] nir: add helpers to check if we can unroll loops

[Erik Faye-Lund]

On Thu, Sep 15, 2016 at 9:03 AM, Timothy Arceri
 wrote:
> This will be used by the loop unroll and lcssa passes.
>
> V2:
> - Check instruction count is not too large for unrolling
> - Add helper for complex loop unrolling
> ---
>  src/compiler/nir/nir.h | 31 +++
>  1 file changed, 31 insertions(+)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 49e8cd8..3a2a13a 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2590,6 +2590,37 @@ bool nir_normalize_cubemap_coords(nir_shader *shader);
>
>  void nir_live_ssa_defs_impl(nir_function_impl *impl);
>
> +static inline bool
> +is_loop_small_enough_to_unroll(nir_shader *shader, nir_loop_info *li)
> +{
> +   unsigned max_iter = shader->options->max_unroll_iterations;
> +
> +   if (li->trip_count > max_iter)
> +  return false;
> +
> +   if (li->force_unroll)
> +  return true;
> +
> +   bool loop_not_too_large =
> +  li->num_instructions * li->trip_count <= max_iter * 25;


"max_iter * 25" seems like a pretty arbirary limit at first glance.
How was it found? Perhaps a comment explaining a bit could be added?

> +static inline bool
> +is_complex_loop(nir_shader *shader, nir_loop_info *li)
> +{
> +   unsigned num_lt = list_length(>loop_terminator_list);
> +   return is_loop_small_enough_to_unroll(shader, li) && num_lt == 2;

Perhaps you could add a comment to explain the "num_lt == 2"-part?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev