<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Dec 27, 2015 at 10:15 AM, Connor Abbott <<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sat, Dec 26, 2015 at 2:09 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> 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. </blockquote><div> </div><div>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. </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div class="h5"> > > 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 > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>