[Mesa-dev] [PATCH] nir: add/export nir_variable_clone()

[Rob Clark]

On Wed, Mar 23, 2016 at 4:55 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> On Mar 23, 2016 1:15 PM, "Rob Clark" <robdclark at gmail.com> wrote:
>>
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>  src/compiler/nir/nir.h       |  1 +
>>  src/compiler/nir/nir_clone.c | 16 ++++++++++++++++
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index 69a3084..724aa77 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -2053,6 +2053,7 @@ 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);
>> +nir_variable * nir_variable_clone(nir_variable *var);
>>
>>  #ifdef DEBUG
>>  void nir_validate_shader(nir_shader *shader);
>> diff --git a/src/compiler/nir/nir_clone.c b/src/compiler/nir/nir_clone.c
>> index 7444dfe..03b4fbd 100644
>> --- a/src/compiler/nir/nir_clone.c
>> +++ b/src/compiler/nir/nir_clone.c
>> @@ -149,6 +149,22 @@ clone_variable(clone_state *state, const nir_variable
>> *var)
>>     return nvar;
>>  }
>>
>> +nir_variable *
>> +nir_variable_clone(nir_variable *var)
>> +{
>> +   clone_state state;
>> +   init_clone_state(&state, false);
>> +
>> +   /* We use the same shader */
>> +   state.ns = ralloc_parent(var);
>> +
>> +   nir_variable *nvar = clone_variable(&state, var);
>
> Do we really need a full-on clone state or can we just make clone_variable
> not take a state?

It does seem a bit overkill, but currently clone_variable needs (ofc)
the memctx, but also does a add_remap() which needs the hashtable..

We *could* pass a bunch more params to clone_variable(), and open-code
the add_remap or make it conditional on hashtable not null or
something like that.  But I guess the clone_shader path into
clone_variable will be the more common-case / critical path thing, so
I didn't mind a bit of extra overhead in construction the unneeded
hashtable for the nir_variable_clone() path..  at least currently this
will just be used for a single pass for inputs/outputs that are
deref'd indirectly.  So I went for simplicity.

BR,
-R

>> +
>> +   free_clone_state(&state);
>> +
>> +   return nvar;
>> +}
>> +
>>  /* clone list of nir_variable: */
>>  static void
>>  clone_var_list(clone_state *state, struct exec_list *dst,
>> --
>> 2.5.0
>>