[Mesa-dev] [PATCH 3/4] nir: support to flatten_all in peephole-select

[Rob Clark]

On Tue, Mar 31, 2015 at 7:08 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Tue, Mar 31, 2015 at 3:57 PM, Rob Clark <robdclark at gmail.com> wrote:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> Freedreno and vc4 want this behavior for the time being (until we have
>> real flow control).  Even after that, we probably want to turn this into
>> some sort of driver tunable threshold, since for at least some hardware,
>> reasonably large if/else is best flattend rather than having divergent
>> flow control.
>>
>> NOTE: wasn't sure about some of the additional restrictions in
>> block_check_for_allowed_instrs()..  are there some other cases where
>> I might need to fix things up in order to be guaranteed to be able to
>> flatten?
>>
>> NOTE: drop vc4 hunk if this is merged first, ofc
>>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>  src/gallium/drivers/vc4/vc4_program.c    |  2 +-
>>  src/glsl/nir/nir.h                       |  2 +-
>>  src/glsl/nir/nir_opt_peephole_select.c   | 21 +++++++++++++++------
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp |  2 +-
>>  4 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/gallium/drivers/vc4/vc4_program.c b/src/gallium/drivers/vc4/vc4_program.c
>> index db51665..09896ce 100644
>> --- a/src/gallium/drivers/vc4/vc4_program.c
>> +++ b/src/gallium/drivers/vc4/vc4_program.c
>> @@ -1673,7 +1673,7 @@ vc4_optimize_nir(struct nir_shader *s)
>>                  progress = nir_copy_prop(s) || progress;
>>                  progress = nir_opt_dce(s) || progress;
>>                  progress = nir_opt_cse(s) || progress;
>> -                progress = nir_opt_peephole_select(s) || progress;
>> +                progress = nir_opt_peephole_select(s, true) || progress;
>
> I really hate boolean arguments. Could we make this parameter a
> "flags" enum and pass a readable name?

hmm, well my thinking was that it someday becomes a threshold (rather
than a boolean) when someone has a clever idea about how to quantify
how aggressive we want the pass to be about flattening things out.  I
don't think that fits very well as a bitmask.  Not sure if that
changes your opinion about using a bitmask.

Possibly a config struct, if we wanted to eventually have multiple
different thresholds (# of instructions, nesting depth, etc) would be
a better option, if you are simply trying to avoid fixing up the
handful of call sites later.

BR,
-R