[Mesa-dev] [PATCH 11/16] nir/lower-io: split out some helper fxns

[Rob Clark]

On Fri, Apr 1, 2016 at 1:57 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> I don't know that I like the lower-io prefix.  Maybe nir/io-to-temp?
> Doesn't really matter
>
> On Sat, Mar 26, 2016 at 2:02 PM, Rob Clark <robdclark at gmail.com> wrote:
>>
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> Prep work to reduce the noise in the next patch.
>>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>  src/compiler/nir/nir_lower_io_to_temporaries.c | 95
>> +++++++++++++++-----------
>>  1 file changed, 57 insertions(+), 38 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir_lower_io_to_temporaries.c
>> b/src/compiler/nir/nir_lower_io_to_temporaries.c
>> index 0db436b..8e9df99 100644
>> --- a/src/compiler/nir/nir_lower_io_to_temporaries.c
>> +++ b/src/compiler/nir/nir_lower_io_to_temporaries.c
>> @@ -35,25 +35,30 @@ struct lower_io_state {
>>  };
>>
>>  static void
>> -emit_output_copies(nir_cursor cursor, struct lower_io_state *state)
>> +emit_copies(nir_cursor cursor, nir_shader *shader, struct exec_list
>> *new_vars,
>> +          struct exec_list *old_vars)
>>  {
>> -   assert(exec_list_length(&state->shader->outputs) ==
>> -          exec_list_length(&state->old_outputs));
>> +   assert(exec_list_length(new_vars) == exec_list_length(old_vars));
>>
>> -   foreach_two_lists(out_node, &state->shader->outputs,
>> -                     temp_node, &state->old_outputs) {
>> -      nir_variable *output = exec_node_data(nir_variable, out_node,
>> node);
>> -      nir_variable *temp = exec_node_data(nir_variable, temp_node, node);
>> +   foreach_two_lists(new_node, new_vars, old_node, old_vars) {
>> +      nir_variable *newv = exec_node_data(nir_variable, new_node, node);
>> +      nir_variable *temp = exec_node_data(nir_variable, old_node, node);
>>
>>        nir_intrinsic_instr *copy =
>> -         nir_intrinsic_instr_create(state->shader,
>> nir_intrinsic_copy_var);
>> -      copy->variables[0] = nir_deref_var_create(copy, output);
>> +         nir_intrinsic_instr_create(shader, nir_intrinsic_copy_var);
>> +      copy->variables[0] = nir_deref_var_create(copy, newv);
>>        copy->variables[1] = nir_deref_var_create(copy, temp);
>>
>>        nir_instr_insert(cursor, &copy->instr);
>>     }
>>  }
>>
>> +static void
>> +emit_output_copies(nir_cursor cursor, struct lower_io_state *state)
>> +{
>> +   emit_copies(cursor, state->shader, &state->shader->outputs,
>> &state->old_outputs);
>> +}
>> +
>>  static bool
>>  emit_output_copies_block(nir_block *block, void *state)
>>  {
>> @@ -69,6 +74,47 @@ emit_output_copies_block(nir_block *block, void *state)
>>     return true;
>>  }
>>
>> +static void
>> +emit_output_copies_impl(nir_function_impl *impl, struct lower_io_state
>> *state)
>
>
> In light of the change to make lower_io_to_temporaries take a nir_function,
> this will have to be reworked a bit.  Perhaps we want an
> emit_gs_output_copies and emit_output_copies?  The GS version would walk the
> entire shader and the non-GS version would take a function impl and just put
> the copies at the end of it.

just catching up on this branch..

but, I have to say, I'm not sure I understand that comment.  That is
what emit_output_copies_impl() already does (and really this patch is
just moving some code, so no functional change compared to what is
already there..)

BR,
-R

> In any case, I like the direction this is going.
>
>>
>> +{
>> +   if (state->shader->stage == MESA_SHADER_GEOMETRY) {
>> +      /* For geometry shaders, we have to emit the output copies right
>> +       * before each EmitVertex call.
>> +       */
>> +      nir_foreach_block(impl, emit_output_copies_block, state);
>> +   } else if (strcmp(impl->function->name, "main") == 0) {
>> +      /* For all other shader types, we need to do the copies right
>> before
>> +       * the jumps to the end block.
>> +       */
>> +      struct set_entry *block_entry;
>> +      set_foreach(impl->end_block->predecessors, block_entry) {
>> +         struct nir_block *block = (void *)block_entry->key;
>> +         emit_output_copies(nir_after_block_before_jump(block), state);
>> +      }
>> +   }
>> +}
>> +
>> +static nir_variable *
>> +create_shadow_temp(struct lower_io_state *state, nir_variable *var)
>> +{
>> +   nir_variable *nvar = ralloc(state->shader, nir_variable);
>> +   memcpy(nvar, var, sizeof *nvar);
>> +
>> +   /* The original is now the temporary */
>> +   nir_variable *temp = var;
>> +
>> +   /* Reparent the name to the new variable */
>> +   ralloc_steal(nvar, nvar->name);
>> +
>> +   /* Give the output a new name with @out-temp appended */
>> +   const char *mode = "out";
>> +   temp->name = ralloc_asprintf(var, "%s@%s-temp", mode, nvar->name);
>> +   temp->data.mode = nir_var_global;
>> +   temp->constant_initializer = NULL;
>> +
>> +   return nvar;
>> +}
>> +
>>  void
>>  nir_lower_io_to_temporaries(nir_shader *shader)
>>  {
>> @@ -84,20 +130,7 @@ nir_lower_io_to_temporaries(nir_shader *shader)
>>      * make a new variable for the actual output.
>>      */
>>     nir_foreach_variable(var, &state.old_outputs) {
>> -      nir_variable *output = ralloc(shader, nir_variable);
>> -      memcpy(output, var, sizeof *output);
>> -
>> -      /* The orignal is now the temporary */
>> -      nir_variable *temp = var;
>> -
>> -      /* Reparent the name to the new variable */
>> -      ralloc_steal(output, output->name);
>> -
>> -      /* Give the output a new name with @out-temp appended */
>> -      temp->name = ralloc_asprintf(var, "%s at out-temp", output->name);
>> -      temp->data.mode = nir_var_global;
>> -      temp->constant_initializer = NULL;
>> -
>> +      nir_variable *output = create_shadow_temp(&state, var);
>>        exec_list_push_tail(&shader->outputs, &output->node);
>>     }
>>
>> @@ -105,21 +138,7 @@ nir_lower_io_to_temporaries(nir_shader *shader)
>>        if (function->impl == NULL)
>>           continue;
>>
>> -      if (shader->stage == MESA_SHADER_GEOMETRY) {
>> -         /* For geometry shaders, we have to emit the output copies right
>> -          * before each EmitVertex call.
>> -          */
>> -         nir_foreach_block(function->impl, emit_output_copies_block,
>> &state);
>> -      } else if (strcmp(function->name, "main") == 0) {
>> -         /* For all other shader types, we need to do the copies right
>> before
>> -          * the jumps to the end block.
>> -          */
>> -         struct set_entry *block_entry;
>> -         set_foreach(function->impl->end_block->predecessors,
>> block_entry) {
>> -            struct nir_block *block = (void *)block_entry->key;
>> -            emit_output_copies(nir_after_block_before_jump(block),
>> &state);
>> -         }
>> -      }
>> +      emit_output_copies_impl(function->impl, &state);
>>
>>        nir_metadata_preserve(function->impl, nir_metadata_block_index |
>>                                              nir_metadata_dominance);
>> --
>> 2.5.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>