[Mesa-dev] [PATCH 7/7] nir: add helper macros for running NIR passes
Rob Clark
robdclark at gmail.com
Wed Oct 28 10:37:44 PDT 2015
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.
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