<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 5, 2016 at 5:12 PM, Timothy Arceri <<a href="mailto:timothy.arceri@collabora.com" target="_blank">timothy.arceri@collabora.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">V2: - updated to create a generic list clone helper nir_cf_list_clone() - continue to assert on clone when fallback flag not set as suggested by Jason. --- src/compiler/nir/nir_clone.c | 58 +++++++++++++++++++++++++++++++------ src/compiler/nir/nir_control_flow.h | 3 ++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/compiler/nir/nir_clone.c b/src/compiler/nir/nir_clone.c index e6483b1..b9b7829 100644 --- a/src/compiler/nir/nir_clone.c +++ b/src/compiler/nir/nir_clone.c @@ -22,7 +22,7 @@ */ #include "nir.h" -#include "nir_control_flow_private.h" +#include "nir_control_flow.h" /* Secret Decoder Ring: * clone_foo(): @@ -35,6 +35,11 @@ typedef struct { /* True if we are cloning an entire shader. */ bool global_clone; + /* This allows us to clone a loop body without having to add srcs from + * outside the loop to the remap table. This is useful for loop unrolling. </blockquote><div> </div><div>It would be better of this comment started with a description of what the variable means rather than a use-case. The other is fine, but we should say some thing such as "allow the clone operation to fall back to the original pointer if no clone pointer is found in the remap table." </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + */ + bool allow_remap_fallback; + /* maps orig ptr -> cloned ptr: */ struct hash_table *remap_table; @@ -46,11 +51,19 @@ typedef struct { } clone_state; static void -init_clone_state(clone_state *state, bool global) +init_clone_state(clone_state *state, struct hash_table *remap_table, + bool global, bool allow_remap_fallback) { state->global_clone = global; - state->remap_table = _mesa_hash_table_create(NULL, _mesa_hash_pointer, - _mesa_key_pointer_equal); + state->allow_remap_fallback = allow_remap_fallback; + + if (remap_table) { + state->remap_table = remap_table; + } else { + state->remap_table = _mesa_hash_table_create(NULL, _mesa_hash_pointer, + _mesa_key_pointer_equal); + } + list_inithead(&state->phi_srcs); } @@ -72,9 +85,10 @@ _lookup_ptr(clone_state *state, const void *ptr, bool global) return (void *)ptr; entry = _mesa_hash_table_search(state->remap_table, ptr); - assert(entry && "Failed to find pointer!"); - if (!entry) - return NULL; + if (!entry) { + assert(state->allow_remap_fallback); + return (void *)ptr; + } return entry->data; } @@ -613,6 +627,32 @@ fixup_phi_srcs(clone_state *state) assert(list_empty(&state->phi_srcs)); } +void +nir_cf_list_clone(nir_cf_list *dst, nir_cf_list *src, nir_cf_node *parent, + struct hash_table *remap_table) +{ + exec_list_make_empty(&dst->list); + dst->impl = src->impl; + + if (exec_list_is_empty(&src->list)) + return; + + clone_state state; + init_clone_state(&state, remap_table, false, true); + + /* We use the same shader */ + state.ns = src->impl->function->shader; + + /* Dest list needs to at least have one block */ </blockquote><div> I'm confused by this. If src->list is empty, then we'll bale above and never get here. Or is this just so that the control-flow code will be happy? If that's the case, perhaps we should say something like "The control-flow code assumes that the list of cf_nodes always starts and ends with a block. We start by adding an empty block." </div><div>With those two comments addressed, 6 and 7 are </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div>This is much nicer. Thank you! </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + nir_block *nblk = nir_block_create(state.ns); + nblk->cf_node.parent = parent; + exec_list_push_tail(&dst->list, &nblk->cf_node.node); </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + + clone_cf_list(&state, &dst->list, &src->list); + + fixup_phi_srcs(&state); +} + static nir_function_impl * clone_function_impl(clone_state *state, const nir_function_impl *fi) { @@ -646,7 +686,7 @@ nir_function_impl * nir_function_impl_clone(const nir_function_impl *fi) { clone_state state; - init_clone_state(&state, false); + init_clone_state(&state, NULL, false, false); /* We use the same shader */ state.ns = fi->function->shader; @@ -686,7 +726,7 @@ nir_shader * nir_shader_clone(void *mem_ctx, const nir_shader *s) { clone_state state; - init_clone_state(&state, true); + init_clone_state(&state, NULL, true, false); nir_shader *ns = nir_shader_create(mem_ctx, s->stage, s->options, NULL); state.ns = ns; diff --git a/src/compiler/nir/nir_control_flow.h b/src/compiler/nir/nir_control_flow.h index b71382f..b496aec 100644 --- a/src/compiler/nir/nir_control_flow.h +++ b/src/compiler/nir/nir_control_flow.h @@ -141,6 +141,9 @@ void nir_cf_reinsert(nir_cf_list *cf_list, nir_cursor cursor); void nir_cf_delete(nir_cf_list *cf_list); +void nir_cf_list_clone(nir_cf_list *dst, nir_cf_list *src, nir_cf_node *parent, + struct hash_table *remap_table); + static inline void nir_cf_list_extract(nir_cf_list *extracted, struct exec_list *cf_list) { -- 2.7.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>