On Mon, 2016-08-29 at 21:06 -0400, Connor Abbott wrote:
> So, you've noticed that your method of handling phi's while cloning
> doesn't handle phi's that point to other phi's in the same block.
> Particularly, something like:
>
> a = phi(b, ...)
> b = phi(a, ...)
>
> which is supposed to swap a and b each iteration. Here's a better
> strategy which should be simpler than this and fixes that problem.
> Create a new remap_table for each iteration of the loop. On the first
> iteration, make all the phi nodes remap to their pre-loop sources
> before cloning the body. On each successive iteration, make each phi
> node map to what the *old* remap table (from the previous iteration)
> says that it's source from the previous iteration remaps to. In
> pseudocode:
>
> nir_cf_list loop_body;
> extract loop body into loop_body
> move loop header instructions (minus phi nodes) into loop_body
>
> struct hash_table *old_remap_table = NULL, *remap_table;
> for each iteration:
> remap_table = new dictionary;
> for each phi in original loop:
> if old_remap_table:
> remap_table[phi] = old_remap_table[phi source pointing to
> end of loop]
> else:
> remap_table[phi] = phi source pointing to block before
> loop
> nir_cf_list cloned_body;
> clone loop_body to cloned_body using remap_table
> insert cloned_body before loop
>
> delete loop_body
> delete loop
I see what you are getting at but what about something like this?
int j = 0;
for (int i=0 ; i < 5; i++) {
...
j++;
if (uniform)
j += 2;
}
I'm not sure we can throw away phis so easily.
>
> This should eliminate the need to expose nir_cf_list_cleanup(),
> stitch_blocks(), etc. publicly as well as the previous patch, and it
> should handle the phi swapping case correctly.
>
>
> On Mon, Aug 29, 2016 at 12:59 AM, Timothy Arceri
> <timothy.arc...@collabora.com> wrote:
> >
> > ---
> > src/compiler/nir/nir.h | 3 +++
> > src/compiler/nir/nir_clone.c | 64
> > +++++++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 60 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > index 0ab3ebc..9083bd0 100644
> > --- a/src/compiler/nir/nir.h
> > +++ b/src/compiler/nir/nir.h
> > @@ -2372,6 +2372,9 @@ void nir_print_instr(const nir_instr *instr,
> > FILE *fp);
> >
> > nir_shader *nir_shader_clone(void *mem_ctx, const nir_shader *s);
> > nir_function_impl *nir_function_impl_clone(const nir_function_impl
> > *fi);
> > +void nir_clone_loop_list(struct exec_list *dst, const struct
> > exec_list *list,
> > + struct hash_table *remap_table,
> > + struct hash_table *phi_remap, nir_shader
> > *ns);
> > nir_constant *nir_constant_clone(const nir_constant *c,
> > nir_variable *var);
> > nir_variable *nir_variable_clone(const nir_variable *c, nir_shader
> > *shader);
> >
> > diff --git a/src/compiler/nir/nir_clone.c
> > b/src/compiler/nir/nir_clone.c
> > index 8808333..071afc9 100644
> > --- a/src/compiler/nir/nir_clone.c
> > +++ b/src/compiler/nir/nir_clone.c
> > @@ -35,9 +35,17 @@ 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.
> > + */
> > + bool allow_remap_fallback;
> > +
> > /* maps orig ptr -> cloned ptr: */
> > struct hash_table *remap_table;
> >
> > + /* used for remaping when cloning loop body for loop unrolling
> > */
> > + struct hash_table *phi_remap_table;
> > +
> > /* List of phi sources. */
> > struct list_head phi_srcs;
> >
> > @@ -46,11 +54,20 @@ 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;
> > +
> > + state->phi_remap_table = NULL;
> > + if (remap_table) {
> > + state->remap_table = remap_table;
> > + } else {
> > + state->remap_table = _mesa_hash_table_create(NULL,
> > _mesa_hash_pointer,
> > + _mesa_key_point
> > er_equal);
> > + }
> > +
> > list_inithead(&state->phi_srcs);
> > }
> >
> > @@ -72,16 +89,32 @@ _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;
> > + return state->allow_remap_fallback ? (void *)ptr : NULL;
> >
> > return entry->data;
> > }
> >
> > +/**
> > + * Updates a phi remap table used for unrolling loops.
> > + */
> > +static void
> > +update_phi_remap_table(clone_state *state, const void *ptr, void
> > *nptr)
> > +{
> > + if (state->phi_remap_table == NULL)
> > + return;
> > +
> > + struct hash_entry *hte;
> > + hash_table_foreach(state->phi_remap_table, hte) {
> > + if (hte->data == ptr)
> > + hte->data = nptr;
> > + }
> > +}
> > +
> > static void
> > add_remap(clone_state *state, void *nptr, const void *ptr)
> > {
> > + update_phi_remap_table(state, ptr, nptr);
> > _mesa_hash_table_insert(state->remap_table, ptr, nptr);
> > }
> >
> > @@ -613,6 +646,23 @@ fixup_phi_srcs(clone_state *state)
> > assert(list_empty(&state->phi_srcs));
> > }
> >
> > +void
> > +nir_clone_loop_list(struct exec_list *dst, const struct exec_list
> > *list,
> > + struct hash_table *remap_table,
> > + struct hash_table *phi_remap, nir_shader *ns)
> > +{
> > + clone_state state;
> > + init_clone_state(&state, remap_table, false, true);
> > +
> > + /* We use the same shader */
> > + state.ns = ns;
> > + state.phi_remap_table = phi_remap;
> > +
> > + clone_cf_list(&state, dst, list);
> > +
> > + fixup_phi_srcs(&state);
> > +}
> > +
> > static nir_function_impl *
> > clone_function_impl(clone_state *state, const nir_function_impl
> > *fi)
> > {
> > @@ -646,7 +696,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 +736,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);
> > state.ns = ns;
> > --
> > 2.7.4
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
