[Mesa-dev] [PATCH 08/11] nir: add helper for cloning loops

[Jason Ekstrand]

On Wed, Oct 5, 2016 at 11:49 AM, Jason Ekstrand <jason at jlekstrand.net>
wrote:

>
>
> On Fri, Sep 16, 2016 at 6:24 AM, Timothy Arceri <
> timothy.arceri at collabora.com> wrote:
>
>> ---
>>  src/compiler/nir/nir.h       |  2 ++
>>  src/compiler/nir/nir_clone.c | 41 ++++++++++++++++++++++++++++++
>> ++++-------
>>  2 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index 29a6f45..d052cad 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -2338,6 +2338,8 @@ 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, 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..7afccbb 100644
>> --- a/src/compiler/nir/nir_clone.c
>> +++ b/src/compiler/nir/nir_clone.c
>> @@ -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.
>> +    */
>> +   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,8 @@ _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;
>>
>
> How about:
>
> if (!entry) {
>    assert(state->allow_remap_fallback);
>    return (void *)ptr;
> }
>
> That avoids an extra bit of control flow and keeps us asserting in the
> regular "clone a shader" case.
>
>
>>
>>     return entry->data;
>>  }
>> @@ -613,6 +625,21 @@ 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,
>>
>
> Maybe take a nir_loop instead of an exec_list.  It would give us a little
> type-saftey
>

Ok, I see why you make it take an exec list now.  Maybe it would be good to
make it take a nir_cf_list instead.  That would get you the shader for free
through nir_cf_list::impl.


>
>
>> +                    struct hash_table *remap_table, nir_shader *ns)
>>
>
> You could dig the nir_shader out of the loop with
> nir_cf_node_get_function()
>
>
>> +{
>> +   clone_state state;
>> +   init_clone_state(&state, remap_table, false, true);
>> +
>> +   /* We use the same shader */
>> +   state.ns = ns;
>> +
>> +   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 +673,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 +713,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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161005/51b3c17a/attachment.html>