[Mesa-dev] [RFC 12/12] nir: Add a pass to inline functions
Connor Abbott
cwabbott0 at gmail.com
Sun Dec 27 14:20:56 PST 2015
On Sun, Dec 27, 2015 at 4:35 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Sun, Dec 27, 2015 at 10:15 AM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>
>> On Sat, Dec 26, 2015 at 2:09 PM, Jason Ekstrand <jason at jlekstrand.net>
>> wrote:
>> > This commit adds a new NIR pass that lowers all function calls away by
>> > inlining the functions.
>> > ---
>> >
>> > There are still two things missing here:
>> >
>> > 1) We really shouldn't do the inline recursively. We should keep a
>> > hash-table of inlined versions or something.
>>
>> I don't think we need anything so complicated. We're collapsing the
>> DAG of functions, so we just need a simple DFS: we should inline the
>> callees before inlining the caller. We could keep a record of the
>> already-inlined functions, but if we visit an already-inlined function
>> we wouldn't do anything so it would only save us the cost of walking
>> over that function.
>
>
> Yes, that's what we want to do for the "inline the whole shader" case. What
> if we want only want to inline part of it? Would that ever seriously
> happen? I don't know. We probably also want to delete all of the "extra"
> functions while we're at it. How do we specify the unique entrypoint? No,
> "main" isn't sufficient.
> --Jason
If we only want to inline some of the functions, then I think the best
thing to do would be similar to what we do with loop unrolling -- when
we run into a function call, first we recurse into the function
callee, and then after we're done we decide whether we want to inline
the callee into the caller based on some heuristic (is this only used
once, how large is it, etc.). Recording which functions we've already
processed becomes more important, since functions we've only processed
might not be nodes in the DAG anymore. This should be pretty
straightforward to add on top of what I suggested earlier, but we
can't handle function calls yet in the backend so we shouldn't worry
too much about this.
We don't need to specify the unique entrypoint at all at this point.
If there are multiple entrypoints, then there are multiple starting
points/parents in the DAG and everything should work fine. We inline
everything into the entrypoints so that they're the only thing
remaining. If we don't want to inline everything, then there are some
more complications -- for example, a function might get used by two
different entrypoints but since it's only used once by each entrypoint
we still want to inline it -- but we can leave that for later.
Deleting functions should probably happen in a separate pass from
inlining. It's pretty similar to DCE, except that since the call graph
is a DAG it's even easier. As for actually deleting the function impl,
we might need a special thing for that since there are a few things we
need to cleanup (the main thing that comes to mind is global register
defs/uses) but otherwise we can just remove it from the list. I would
count this as more of a compile-time reduction thing, though, since
when we get to the backend we're going to throw away everything but
the specified entrypoint anyways (and the functions it depends on
recursively, if we get around to doing real function support).
>
>>
>>
>> >
>> > 2) It doesn't properly handle things like samplers as parameters.
>> > Since
>> > those can only be "in" parameters, I had thought about treating the
>> > variable as a "proxy" and going through and rewriting all of the
>> > derefs
>> > that use it. We could do the same thing for regular parameters when
>> > a
>> > shadow isn't needed.
>> >
>> > Still, I thought it was worth sending out because it does work for most
>> > cases.
>> >
>> > src/glsl/Makefile.sources | 1 +
>> > src/glsl/nir/nir.h | 3 +
>> > src/glsl/nir/nir_inline_functions.c | 138
>> > ++++++++++++++++++++++++++++++++++++
>> > 3 files changed, 142 insertions(+)
>> > create mode 100644 src/glsl/nir/nir_inline_functions.c
>> >
>> > diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
>> > index fa3868c..c271afd 100644
>> > --- a/src/glsl/Makefile.sources
>> > +++ b/src/glsl/Makefile.sources
>> > @@ -35,6 +35,7 @@ NIR_FILES = \
>> > nir/nir_dominance.c \
>> > nir/nir_from_ssa.c \
>> > nir/nir_gs_count_vertices.c \
>> > + nir/nir_inline_functions.c \
>> > nir/nir_intrinsics.c \
>> > nir/nir_intrinsics.h \
>> > nir/nir_instr_set.c \
>> > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> > index 2e29617..437f63a 100644
>> > --- a/src/glsl/nir/nir.h
>> > +++ b/src/glsl/nir/nir.h
>> > @@ -1938,6 +1938,9 @@ bool nir_split_var_copies(nir_shader *shader);
>> > bool nir_lower_returns_impl(nir_function_impl *impl);
>> > bool nir_lower_returns(nir_shader *shader);
>> >
>> > +bool nir_inline_functions_impl(nir_function_impl *impl);
>> > +bool nir_inline_functions(nir_shader *shader);
>> > +
>> > void nir_lower_var_copy_instr(nir_intrinsic_instr *copy, void
>> > *mem_ctx);
>> > void nir_lower_var_copies(nir_shader *shader);
>> >
>> > diff --git a/src/glsl/nir/nir_inline_functions.c
>> > b/src/glsl/nir/nir_inline_functions.c
>> > new file mode 100644
>> > index 0000000..e7e17ca
>> > --- /dev/null
>> > +++ b/src/glsl/nir/nir_inline_functions.c
>> > @@ -0,0 +1,138 @@
>> > +/*
>> > + * Copyright © 2015 Intel Corporation
>> > + *
>> > + * Permission is hereby granted, free of charge, to any person
>> > obtaining a
>> > + * copy of this software and associated documentation files (the
>> > "Software"),
>> > + * to deal in the Software without restriction, including without
>> > limitation
>> > + * the rights to use, copy, modify, merge, publish, distribute,
>> > sublicense,
>> > + * and/or sell copies of the Software, and to permit persons to whom
>> > the
>> > + * Software is furnished to do so, subject to the following conditions:
>> > + *
>> > + * The above copyright notice and this permission notice (including the
>> > next
>> > + * paragraph) shall be included in all copies or substantial portions
>> > of the
>> > + * Software.
>> > + *
>> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> > EXPRESS OR
>> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> > MERCHANTABILITY,
>> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> > SHALL
>> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> > OTHER
>> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> > ARISING
>> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> > DEALINGS
>> > + * IN THE SOFTWARE.
>> > + */
>> > +
>> > +#include "nir.h"
>> > +#include "nir_builder.h"
>> > +#include "nir_control_flow.h"
>> > +
>> > +struct inline_functions_state {
>> > + nir_function_impl *impl;
>> > + nir_builder builder;
>> > + bool progress;
>> > +};
>> > +
>> > +static bool
>> > +inline_functions_block(nir_block *block, void *void_state)
>> > +{
>> > + struct inline_functions_state *state = void_state;
>> > +
>> > + nir_builder *b = &state->builder;
>> > +
>> > + /* This is tricky. We're iterating over instructions in a block
>> > but, as
>> > + * we go, the block and its instruction list are being split into
>> > + * pieces. However, this *should* be safe since foreach_safe always
>> > + * stashes the next thing in the iteration. That next thing will
>> > + * properly get moved to the next block when it gets split, and we
>> > + * continue iterating there.
>> > + */
>> > + nir_foreach_instr_safe(block, instr) {
>> > + if (instr->type != nir_instr_type_call)
>> > + continue;
>> > +
>> > + state->progress = true;
>> > +
>> > + nir_call_instr *call = nir_instr_as_call(instr);
>> > + assert(call->callee->impl);
>> > +
>> > + nir_function_impl *callee_copy =
>> > + nir_function_impl_clone(call->callee->impl);
>> > +
>> > + exec_list_append(&state->impl->locals, &callee_copy->locals);
>> > + exec_list_append(&state->impl->registers,
>> > &callee_copy->registers);
>> > +
>> > + b->cursor = nir_before_instr(&call->instr);
>> > +
>> > + /* Add copies of all in parameters */
>> > + assert(call->num_params == callee_copy->num_params);
>> > + for (unsigned i = 0; i < callee_copy->num_params; i++) {
>> > + /* Only in or inout parameters */
>> > + if (call->callee->params[i].param_type == nir_parameter_out)
>> > + continue;
>> > +
>> > + nir_copy_deref_var(b, nir_deref_var_create(b->shader,
>> > +
>> > callee_copy->params[i]),
>> > + call->params[i]);
>> > + }
>> > +
>> > + /* Pluck the body out of the function and place it here */
>> > + nir_cf_list body;
>> > + nir_cf_list_extract(&body, &callee_copy->body);
>> > + nir_cf_reinsert(&body, b->cursor);
>> > +
>> > + b->cursor = nir_before_instr(&call->instr);
>> > +
>> > + /* Add copies of all out parameters and the return */
>> > + assert(call->num_params == callee_copy->num_params);
>> > + for (unsigned i = 0; i < callee_copy->num_params; i++) {
>> > + /* Only out or inout parameters */
>> > + if (call->callee->params[i].param_type == nir_parameter_in)
>> > + continue;
>> > +
>> > + nir_copy_deref_var(b, call->params[i],
>> > + nir_deref_var_create(b->shader,
>> > +
>> > callee_copy->params[i]));
>> > + }
>> > + if (!glsl_type_is_void(call->callee->return_type)) {
>> > + nir_copy_deref_var(b, call->return_deref,
>> > + nir_deref_var_create(b->shader,
>> > +
>> > callee_copy->return_var));
>> > + }
>> > +
>> > + nir_instr_remove(&call->instr);
>> > + }
>> > +
>> > + return true;
>> > +}
>> > +
>> > +bool
>> > +nir_inline_functions_impl(nir_function_impl *impl)
>> > +{
>> > + struct inline_functions_state state;
>> > +
>> > + state.progress = false;
>> > + state.impl = impl;
>> > + nir_builder_init(&state.builder, impl);
>> > +
>> > + nir_foreach_block(impl, inline_functions_block, &state);
>> > +
>> > + /* SSA and register indices are completely messed up now */
>> > + nir_index_ssa_defs(impl);
>> > + nir_index_local_regs(impl);
>> > +
>> > + nir_metadata_preserve(impl, nir_metadata_none);
>> > +
>> > + return state.progress;
>> > +}
>> > +
>> > +bool
>> > +nir_inline_functions(nir_shader *shader)
>> > +{
>> > + bool progress = false;
>> > +
>> > + nir_foreach_function(shader, function) {
>> > + if (function->impl)
>> > + progress = nir_inline_functions_impl(function->impl) ||
>> > progress;
>> > + }
>> > +
>> > + return progress;
>> > +}
>> > --
>> > 2.5.0.400.gff86faf
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
More information about the mesa-dev
mailing list