[Mesa-dev] [PATCH 7/7] nir: add helper macros for running NIR passes
Jason Ekstrand
jason at jlekstrand.net
Wed Oct 28 12:40:42 PDT 2015
On Wed, Oct 28, 2015 at 10:37 AM, Rob Clark <robdclark at gmail.com> wrote:
> On Wed, Oct 28, 2015 at 12:09 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Sat, Oct 24, 2015 at 10:08 AM, Rob Clark <robdclark at gmail.com> wrote:
>>> From: Rob Clark <robclark at freedesktop.org>
>>>
>>> Convenient place to put in some extra sanity checking, without making
>>> things messy for the drivers running the passes.
>>
>> In the short-term this seems to work (at least for testing nir_clone).
>> In the long-term, I'm not sure that a macro is really what we want.
>> I've mentioned a time or two before that what I *think* I'd like to do
>> (don't know exactly how it will work out yet) is to have a little
>> datastructure
>>
>> typedef struct nir_pass {
>> bool (*shader_pass_func)(nir_shader *shader, void *data);
>> bool (*impl_pass_func)(nir_function_impl *impl, void *data);
>> nir_metadata metadata_preserved;
>> void *data;
>> } nir_pass;
>>
>> and have each of the passes expose one of these as a const global
>> variable instead of exposing the actual functions. Then we would have
>> a runner function (or macro) that could run a pass. The runner would
>> take care of validation, trashing metadata, and maybe even cloning.
>> If no shader_pass_func is provided but you call it on a shader, the
>> runner would iterate over all of the overloads for you and run the
>> impl_pass_func on each. We could also have helpers that take an array
>> and run all of them or even take an array and run it in a loop until
>> no more progress is made.
>
> meh, once we collapse the run+validate into a single line macro call,
> having list of calls sounds like it doesn't really take up more lines
> of code compared to a table of nir passes.. plus old fashioned code
> has a lot more flexibility without having to reinvent loops and ifs
> and that sort of thing. Keep in mind some passes are conditional on
> draw state (ie. what we are lowering) or shader stage, etc.
Really, colapsing run+validate is only part of the desire there. The
bigger thing is that it moves metadata invalidation out of the passes.
This makes it much harder to forget (I found 3 or 4 passes that do it
wrong today).
This really doesn't change what you can do. If you look at my branch
(which I'm about to send out), the only real difference between
NIR_PASS() and nir_shader_run_pass is that one is longer and
lower-case. You can't wholesale replace the shader, but that seems
like something we only care about for testing cloning.
> BR,
> -R
>
>
>> The thing I haven't quite settled on is how to pass extra parameters.
>> For some passes, we could just put the extra stuff in compiler_options
>> but we don't want to litter it too bad. The other option is to do
>> what I did above and use the classic void pointer. Then drivers would
>> have to just make a copy and set the data pointer to whatever they
>> want.
>>
>> Maybe I should just go implement this...
>>
>>> TODO: convert ir3/vc4..
>>>
>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>> ---
>>> src/glsl/nir/nir.h | 33 ++++++++++
>>> src/mesa/drivers/dri/i965/brw_nir.c | 126 ++++++++++++------------------------
>>> 2 files changed, 76 insertions(+), 83 deletions(-)
>>>
>>> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>>> index 3ab720b..053420d 100644
>>> --- a/src/glsl/nir/nir.h
>>> +++ b/src/glsl/nir/nir.h
>>> @@ -1939,10 +1939,43 @@ nir_shader_mutable(nir_shader *shader)
>>>
>>> #ifdef DEBUG
>>> void nir_validate_shader(nir_shader *shader);
>>> +static inline bool
>>> +__nir_test_clone(void)
>>> +{
>>> + static int enable = -1;
>>> + if (enable == -1) {
>>> + enable = (getenv("NIR_TEST_CLONE") == NULL) ? 0 : 1;
>>> + }
>>> + return enable == 1;
>>> +}
>>> #else
>>> static inline void nir_validate_shader(nir_shader *shader) { (void) shader; }
>>> +static inline bool __nir_test_clone(void) { return false; }
>>> #endif /* DEBUG */
>>>
>>> +
>>> +#define NIR_PASS(pass, nir, ...) ({ \
>>> + assert(nir_shader_is_mutable(nir)); \
>>> + pass(nir, ##__VA_ARGS__); \
>>> + nir_validate_shader(nir); \
>>> + if (__nir_test_clone()) { \
>>> + nir = nir_shader_clone(ralloc_parent(nir), nir); \
>>> + nir_validate_shader(nir); \
>>> + } \
>>> + })
>>> +
>>> +#define NIR_PASS_PROGRESS(pass, nir, ...) ({ \
>>> + assert(nir_shader_is_mutable(nir)); \
>>> + bool __ret = pass(nir, ##__VA_ARGS__); \
>>> + nir_validate_shader(nir); \
>>> + if (__nir_test_clone()) { \
>>> + nir = nir_shader_clone(ralloc_parent(nir), nir); \
>>> + nir_validate_shader(nir); \
>>> + } \
>>> + __ret; \
>>> + })
>>> +
>>> +
>>> void nir_calc_dominance_impl(nir_function_impl *impl);
>>> void nir_calc_dominance(nir_shader *shader);
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
>>> index 8f09165..bc42fea 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_nir.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
>>> @@ -136,47 +136,34 @@ brw_nir_lower_outputs(nir_shader *nir, bool is_scalar)
>>> }
>>> }
>>>
>>> -static void
>>> +static nir_shader *
>>> nir_optimize(nir_shader *nir, bool is_scalar)
>>> {
>>> bool progress;
>>> do {
>>> progress = false;
>>> - nir_lower_vars_to_ssa(nir);
>>> - nir_validate_shader(nir);
>>>
>>> - if (is_scalar) {
>>> - nir_lower_alu_to_scalar(nir);
>>> - nir_validate_shader(nir);
>>> - }
>>> + NIR_PASS(nir_lower_vars_to_ssa, nir);
>>>
>>> - progress |= nir_copy_prop(nir);
>>> - nir_validate_shader(nir);
>>> + if (is_scalar)
>>> + NIR_PASS(nir_lower_alu_to_scalar, nir);
>>>
>>> - if (is_scalar) {
>>> - nir_lower_phis_to_scalar(nir);
>>> - nir_validate_shader(nir);
>>> - }
>>> + progress |= NIR_PASS_PROGRESS(nir_copy_prop, nir);
>>> +
>>> + if (is_scalar)
>>> + NIR_PASS(nir_lower_phis_to_scalar, nir);
>>>
>>> - progress |= nir_copy_prop(nir);
>>> - nir_validate_shader(nir);
>>> - progress |= nir_opt_dce(nir);
>>> - nir_validate_shader(nir);
>>> - progress |= nir_opt_cse(nir);
>>> - nir_validate_shader(nir);
>>> - progress |= nir_opt_peephole_select(nir);
>>> - nir_validate_shader(nir);
>>> - progress |= nir_opt_algebraic(nir);
>>> - nir_validate_shader(nir);
>>> - progress |= nir_opt_constant_folding(nir);
>>> - nir_validate_shader(nir);
>>> - progress |= nir_opt_dead_cf(nir);
>>> - nir_validate_shader(nir);
>>> - progress |= nir_opt_remove_phis(nir);
>>> - nir_validate_shader(nir);
>>> - progress |= nir_opt_undef(nir);
>>> - nir_validate_shader(nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_copy_prop, nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_opt_dce, nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_opt_cse, nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_opt_peephole_select, nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_opt_algebraic, nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_opt_constant_folding, nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_opt_dead_cf, nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_opt_remove_phis, nir);
>>> + progress |= NIR_PASS_PROGRESS(nir_opt_undef, nir);
>>> } while (progress);
>>> + return nir;
>>> }
>>>
>>> nir_shader *
>>> @@ -204,75 +191,52 @@ brw_create_nir(struct brw_context *brw,
>>> }
>>> nir_validate_shader(nir);
>>>
>>> - if (stage == MESA_SHADER_GEOMETRY) {
>>> - nir_lower_gs_intrinsics(nir);
>>> - nir_validate_shader(nir);
>>> - }
>>> + if (stage == MESA_SHADER_GEOMETRY)
>>> + NIR_PASS(nir_lower_gs_intrinsics, nir);
>>>
>>> - nir_lower_global_vars_to_local(nir);
>>> - nir_validate_shader(nir);
>>> -
>>> - nir_lower_tex(nir, &tex_options);
>>> - nir_validate_shader(nir);
>>> -
>>> - nir_normalize_cubemap_coords(nir);
>>> - nir_validate_shader(nir);
>>> -
>>> - nir_split_var_copies(nir);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_lower_global_vars_to_local, nir);
>>> + NIR_PASS(nir_lower_tex, nir, &tex_options);
>>> + NIR_PASS(nir_normalize_cubemap_coords, nir);
>>> + NIR_PASS(nir_split_var_copies, nir);
>>>
>>> - nir_optimize(nir, is_scalar);
>>> + nir = nir_optimize(nir, is_scalar);
>>>
>>> /* Lower a bunch of stuff */
>>> - nir_lower_var_copies(nir);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_lower_var_copies, nir);
>>>
>>> /* Get rid of split copies */
>>> - nir_optimize(nir, is_scalar);
>>> + nir = nir_optimize(nir, is_scalar);
>>>
>>> brw_nir_lower_inputs(nir, is_scalar);
>>> brw_nir_lower_outputs(nir, is_scalar);
>>> nir_assign_var_locations(&nir->uniforms,
>>> &nir->num_uniforms,
>>> is_scalar ? type_size_scalar : type_size_vec4);
>>> - nir_lower_io(nir, nir_var_all,
>>> + NIR_PASS(nir_lower_io, nir, nir_var_all,
>>> is_scalar ? type_size_scalar : type_size_vec4);
>>> - nir_validate_shader(nir);
>>> -
>>> - nir_remove_dead_variables(nir);
>>> - nir_validate_shader(nir);
>>>
>>> - if (shader_prog) {
>>> - nir_lower_samplers(nir, shader_prog);
>>> - nir_validate_shader(nir);
>>> - }
>>> + NIR_PASS(nir_remove_dead_variables, nir);
>>>
>>> - nir_lower_system_values(nir);
>>> - nir_validate_shader(nir);
>>> + if (shader_prog)
>>> + NIR_PASS(nir_lower_samplers, nir, shader_prog);
>>>
>>> - nir_lower_atomics(nir);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_lower_system_values, nir);
>>> + NIR_PASS(nir_lower_atomics, nir);
>>>
>>> - nir_optimize(nir, is_scalar);
>>> + nir = nir_optimize(nir, is_scalar);
>>>
>>> if (brw->gen >= 6) {
>>> /* Try and fuse multiply-adds */
>>> - nir_opt_peephole_ffma(nir);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_opt_peephole_ffma, nir);
>>> }
>>>
>>> - nir_opt_algebraic_late(nir);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_opt_algebraic_late, nir);
>>>
>>> - nir_lower_locals_to_regs(nir);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_lower_locals_to_regs, nir);
>>>
>>> - nir_lower_to_source_mods(nir);
>>> - nir_validate_shader(nir);
>>> - nir_copy_prop(nir);
>>> - nir_validate_shader(nir);
>>> - nir_opt_dce(nir);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_lower_to_source_mods, nir);
>>> + NIR_PASS(nir_copy_prop, nir);
>>> + NIR_PASS(nir_opt_dce, nir);
>>>
>>> if (unlikely(debug_enabled)) {
>>> /* Re-index SSA defs so we print more sensible numbers. */
>>> @@ -286,15 +250,11 @@ brw_create_nir(struct brw_context *brw,
>>> nir_print_shader(nir, stderr);
>>> }
>>>
>>> - nir_convert_from_ssa(nir, true);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_convert_from_ssa, nir, true);
>>>
>>> if (!is_scalar) {
>>> - nir_move_vec_src_uses_to_dest(nir);
>>> - nir_validate_shader(nir);
>>> -
>>> - nir_lower_vec_to_movs(nir);
>>> - nir_validate_shader(nir);
>>> + NIR_PASS(nir_move_vec_src_uses_to_dest, nir);
>>> + NIR_PASS(nir_lower_vec_to_movs, nir);
>>> }
>>>
>>> /* This is the last pass we run before we start emitting stuff. It
>>> --
>>> 2.5.0
>>>
More information about the mesa-dev
mailing list