[Mesa-dev] [PATCH 2/3] nir/print: bit of state refactoring
Rob Clark
robdclark at gmail.com
Tue Sep 15 19:00:19 PDT 2015
On Tue, Sep 15, 2015 at 9:50 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> On Tue, Sep 15, 2015 at 9:42 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Tue, Sep 15, 2015 at 8:48 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>> On Tue, Sep 15, 2015 at 7:33 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>> From: Rob Clark <robclark at freedesktop.org>
>>>>
>>>> Rename print_var_state to print_state, and stuff FILE ptr into the state
>>>> object. This avoids passing around an extra parameter everywhere.
>>>>
>>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>>> ---
>>>> src/glsl/nir/nir_print.c | 95 +++++++++++++++++++++++++++---------------------
>>>> 1 file changed, 54 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/src/glsl/nir/nir_print.c b/src/glsl/nir/nir_print.c
>>>> index 69cadba..405dbf3 100644
>>>> --- a/src/glsl/nir/nir_print.c
>>>> +++ b/src/glsl/nir/nir_print.c
>>>> @@ -37,6 +37,7 @@ print_tabs(unsigned num_tabs, FILE *fp)
>>>> }
>>>>
>>>> typedef struct {
>>>> + FILE *fp;
>>>> /** map from nir_variable -> printable name */
>>>> struct hash_table *ht;
>>>>
>>>> @@ -45,7 +46,7 @@ typedef struct {
>>>>
>>>> /* an index used to make new non-conflicting names */
>>>> unsigned index;
>>>> -} print_var_state;
>>>> +} print_state;
>>>>
>>>> static void
>>>> print_register(nir_register *reg, FILE *fp)
>>>> @@ -206,8 +207,10 @@ print_alu_instr(nir_alu_instr *instr, FILE *fp)
>>>> }
>>>>
>>>> static void
>>>> -print_var_decl(nir_variable *var, print_var_state *state, FILE *fp)
>>>> +print_var_decl(nir_variable *var, print_state *state)
>>>> {
>>>> + FILE *fp = state->fp;
>>>> +
>>>> fprintf(fp, "decl_var ");
>>>>
>>>> const char *const cent = (var->data.centroid) ? "centroid " : "";
>>>> @@ -253,7 +256,7 @@ print_var_decl(nir_variable *var, print_var_state *state, FILE *fp)
>>>> }
>>>>
>>>> static void
>>>> -print_var(nir_variable *var, print_var_state *state, FILE *fp)
>>>> +print_var(nir_variable *var, print_state *state, FILE *fp)
>>>
>>> You should remove the fp argument from here...
>>>
>>>> {
>>>> const char *name;
>>>> if (state) {
>>>> @@ -269,13 +272,13 @@ print_var(nir_variable *var, print_var_state *state, FILE *fp)
>>>> }
>>>>
>>>> static void
>>>> -print_deref_var(nir_deref_var *deref, print_var_state *state, FILE *fp)
>>>> +print_deref_var(nir_deref_var *deref, print_state *state, FILE *fp)
>>>
>>> and here...
>>>
>>>> {
>>>> print_var(deref->var, state, fp);
>>>> }
>>>>
>>>> static void
>>>> -print_deref_array(nir_deref_array *deref, print_var_state *state, FILE *fp)
>>>> +print_deref_array(nir_deref_array *deref, print_state *state, FILE *fp)
>>>
>>> and here...
>>>
>>>> {
>>>> fprintf(fp, "[");
>>>> switch (deref->deref_array_type) {
>>>> @@ -296,13 +299,13 @@ print_deref_array(nir_deref_array *deref, print_var_state *state, FILE *fp)
>>>>
>>>> static void
>>>> print_deref_struct(nir_deref_struct *deref, const struct glsl_type *parent_type,
>>>> - print_var_state *state, FILE *fp)
>>>> + print_state *state, FILE *fp)
>>>
>>> and here...
>>>
>>>> {
>>>> fprintf(fp, ".%s", glsl_get_struct_elem_name(parent_type, deref->index));
>>>> }
>>>>
>>>> static void
>>>> -print_deref(nir_deref_var *deref, print_var_state *state, FILE *fp)
>>>> +print_deref(nir_deref_var *deref, print_state *state, FILE *fp)
>>>
>>> and here...
>>>
>>>> {
>>>> nir_deref *tail = &deref->deref;
>>>> nir_deref *pretail = NULL;
>>>> @@ -335,7 +338,7 @@ print_deref(nir_deref_var *deref, print_var_state *state, FILE *fp)
>>>> }
>>>>
>>>> static void
>>>> -print_intrinsic_instr(nir_intrinsic_instr *instr, print_var_state *state,
>>>> +print_intrinsic_instr(nir_intrinsic_instr *instr, print_state *state,
>>>> FILE *fp)
>>>
>>> and here...
>>>
>>>> {
>>>> unsigned num_srcs = nir_intrinsic_infos[instr->intrinsic].num_srcs;
>>>> @@ -380,7 +383,7 @@ print_intrinsic_instr(nir_intrinsic_instr *instr, print_var_state *state,
>>>> }
>>>>
>>>> static void
>>>> -print_tex_instr(nir_tex_instr *instr, print_var_state *state, FILE *fp)
>>>> +print_tex_instr(nir_tex_instr *instr, print_state *state, FILE *fp)
>>>> {
>>>> print_dest(&instr->dest, fp);
>>>>
>>>> @@ -499,7 +502,7 @@ print_tex_instr(nir_tex_instr *instr, print_var_state *state, FILE *fp)
>>>> }
>>>>
>>>> static void
>>>> -print_call_instr(nir_call_instr *instr, print_var_state *state, FILE *fp)
>>>> +print_call_instr(nir_call_instr *instr, print_state *state, FILE *fp)
>>>
>>> and here. Either doing the entire refactor or leaving it alone is
>>> better than leaving things in an inconsistent state.
>>
>> well, it is slightly less straightforward than you think, since
>> nir_print_instr() calls into some of these w/ null state but non-null
>> fp..
>>
>> probably some more of them can be converted over over with a bit more
>> time spent untangling it.. and/or I can re-work nir_print_instr() to
>> use the state object.. but I was trying to avoid re-writing the whole
>> thing..
>
> Ah, right. I would just have nir_print_instr create a state with a
> NULL hash table and then change the check to if (state->ht). It made a
> lot more sense to pass through a NULL state when the state only
> pertained to variables, but here you've changed it to store the fp as
> well.
ok, I can update the patch to use state for the nir_print_instr() path
too.. it ends up being a bit larger patch, but I guess the end result
is for the better..
BR,
-R
>>
>> BR,
>> -R
>>
>>>
>>>> {
>>>> fprintf(fp, "call %s ", instr->callee->function->name);
>>>>
>>>> @@ -594,7 +597,7 @@ print_parallel_copy_instr(nir_parallel_copy_instr *instr, FILE *fp)
>>>> }
>>>>
>>>> static void
>>>> -print_instr(const nir_instr *instr, print_var_state *state, unsigned tabs, FILE *fp)
>>>> +print_instr(const nir_instr *instr, print_state *state, unsigned tabs, FILE *fp)
>>>> {
>>>> print_tabs(tabs, fp);
>>>>
>>>> @@ -650,12 +653,14 @@ compare_block_index(const void *p1, const void *p2)
>>>> return (int) block1->index - (int) block2->index;
>>>> }
>>>>
>>>> -static void print_cf_node(nir_cf_node *node, print_var_state *state,
>>>> - unsigned tabs, FILE *fp);
>>>> +static void print_cf_node(nir_cf_node *node, print_state *state,
>>>> + unsigned tabs);
>>>>
>>>> static void
>>>> -print_block(nir_block *block, print_var_state *state, unsigned tabs, FILE *fp)
>>>> +print_block(nir_block *block, print_state *state, unsigned tabs)
>>>> {
>>>> + FILE *fp = state->fp;
>>>> +
>>>> print_tabs(tabs, fp);
>>>> fprintf(fp, "block block_%u:\n", block->index);
>>>>
>>>> @@ -697,51 +702,54 @@ print_block(nir_block *block, print_var_state *state, unsigned tabs, FILE *fp)
>>>> }
>>>>
>>>> static void
>>>> -print_if(nir_if *if_stmt, print_var_state *state, unsigned tabs, FILE *fp)
>>>> +print_if(nir_if *if_stmt, print_state *state, unsigned tabs)
>>>> {
>>>> + FILE *fp = state->fp;
>>>> +
>>>> print_tabs(tabs, fp);
>>>> fprintf(fp, "if ");
>>>> print_src(&if_stmt->condition, fp);
>>>> fprintf(fp, " {\n");
>>>> foreach_list_typed(nir_cf_node, node, node, &if_stmt->then_list) {
>>>> - print_cf_node(node, state, tabs + 1, fp);
>>>> + print_cf_node(node, state, tabs + 1);
>>>> }
>>>> print_tabs(tabs, fp);
>>>> fprintf(fp, "} else {\n");
>>>> foreach_list_typed(nir_cf_node, node, node, &if_stmt->else_list) {
>>>> - print_cf_node(node, state, tabs + 1, fp);
>>>> + print_cf_node(node, state, tabs + 1);
>>>> }
>>>> print_tabs(tabs, fp);
>>>> fprintf(fp, "}\n");
>>>> }
>>>>
>>>> static void
>>>> -print_loop(nir_loop *loop, print_var_state *state, unsigned tabs, FILE *fp)
>>>> +print_loop(nir_loop *loop, print_state *state, unsigned tabs)
>>>> {
>>>> + FILE *fp = state->fp;
>>>> +
>>>> print_tabs(tabs, fp);
>>>> fprintf(fp, "loop {\n");
>>>> foreach_list_typed(nir_cf_node, node, node, &loop->body) {
>>>> - print_cf_node(node, state, tabs + 1, fp);
>>>> + print_cf_node(node, state, tabs + 1);
>>>> }
>>>> print_tabs(tabs, fp);
>>>> fprintf(fp, "}\n");
>>>> }
>>>>
>>>> static void
>>>> -print_cf_node(nir_cf_node *node, print_var_state *state, unsigned int tabs,
>>>> - FILE *fp)
>>>> +print_cf_node(nir_cf_node *node, print_state *state, unsigned int tabs)
>>>> {
>>>> switch (node->type) {
>>>> case nir_cf_node_block:
>>>> - print_block(nir_cf_node_as_block(node), state, tabs, fp);
>>>> + print_block(nir_cf_node_as_block(node), state, tabs);
>>>> break;
>>>>
>>>> case nir_cf_node_if:
>>>> - print_if(nir_cf_node_as_if(node), state, tabs, fp);
>>>> + print_if(nir_cf_node_as_if(node), state, tabs);
>>>> break;
>>>>
>>>> case nir_cf_node_loop:
>>>> - print_loop(nir_cf_node_as_loop(node), state, tabs, fp);
>>>> + print_loop(nir_cf_node_as_loop(node), state, tabs);
>>>> break;
>>>>
>>>> default:
>>>> @@ -750,8 +758,10 @@ print_cf_node(nir_cf_node *node, print_var_state *state, unsigned int tabs,
>>>> }
>>>>
>>>> static void
>>>> -print_function_impl(nir_function_impl *impl, print_var_state *state, FILE *fp)
>>>> +print_function_impl(nir_function_impl *impl, print_state *state)
>>>> {
>>>> + FILE *fp = state->fp;
>>>> +
>>>> fprintf(fp, "\nimpl %s ", impl->overload->function->name);
>>>>
>>>> for (unsigned i = 0; i < impl->num_params; i++) {
>>>> @@ -772,7 +782,7 @@ print_function_impl(nir_function_impl *impl, print_var_state *state, FILE *fp)
>>>>
>>>> foreach_list_typed(nir_variable, var, node, &impl->locals) {
>>>> fprintf(fp, "\t");
>>>> - print_var_decl(var, state, fp);
>>>> + print_var_decl(var, state);
>>>> }
>>>>
>>>> foreach_list_typed(nir_register, reg, node, &impl->registers) {
>>>> @@ -783,7 +793,7 @@ print_function_impl(nir_function_impl *impl, print_var_state *state, FILE *fp)
>>>> nir_index_blocks(impl);
>>>>
>>>> foreach_list_typed(nir_cf_node, node, node, &impl->body) {
>>>> - print_cf_node(node, state, 1, fp);
>>>> + print_cf_node(node, state, 1);
>>>> }
>>>>
>>>> fprintf(fp, "\tblock block_%u:\n}\n\n", impl->end_block->index);
>>>> @@ -791,8 +801,10 @@ print_function_impl(nir_function_impl *impl, print_var_state *state, FILE *fp)
>>>>
>>>> static void
>>>> print_function_overload(nir_function_overload *overload,
>>>> - print_var_state *state, FILE *fp)
>>>> + print_state *state)
>>>> {
>>>> + FILE *fp = state->fp;
>>>> +
>>>> fprintf(fp, "decl_overload %s ", overload->function->name);
>>>>
>>>> for (unsigned i = 0; i < overload->num_params; i++) {
>>>> @@ -826,22 +838,23 @@ print_function_overload(nir_function_overload *overload,
>>>> fprintf(fp, "\n");
>>>>
>>>> if (overload->impl != NULL) {
>>>> - print_function_impl(overload->impl, state, fp);
>>>> + print_function_impl(overload->impl, state);
>>>> return;
>>>> }
>>>> }
>>>>
>>>> static void
>>>> -print_function(nir_function *func, print_var_state *state, FILE *fp)
>>>> +print_function(nir_function *func, print_state *state)
>>>> {
>>>> foreach_list_typed(nir_function_overload, overload, node, &func->overload_list) {
>>>> - print_function_overload(overload, state, fp);
>>>> + print_function_overload(overload, state);
>>>> }
>>>> }
>>>>
>>>> static void
>>>> -init_print_state(print_var_state *state)
>>>> +init_print_state(print_state *state, nir_shader *shader, FILE *fp)
>>>> {
>>>> + state->fp = fp;
>>>> state->ht = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
>>>> _mesa_key_pointer_equal);
>>>> state->syms = _mesa_set_create(NULL, _mesa_key_hash_string,
>>>> @@ -850,7 +863,7 @@ init_print_state(print_var_state *state)
>>>> }
>>>>
>>>> static void
>>>> -destroy_print_state(print_var_state *state)
>>>> +destroy_print_state(print_state *state)
>>>> {
>>>> _mesa_hash_table_destroy(state->ht, NULL);
>>>> _mesa_set_destroy(state->syms, NULL);
>>>> @@ -859,27 +872,27 @@ destroy_print_state(print_var_state *state)
>>>> void
>>>> nir_print_shader(nir_shader *shader, FILE *fp)
>>>> {
>>>> - print_var_state state;
>>>> - init_print_state(&state);
>>>> + print_state state;
>>>> + init_print_state(&state, shader, fp);
>>>>
>>>> foreach_list_typed(nir_variable, var, node, &shader->uniforms) {
>>>> - print_var_decl(var, &state, fp);
>>>> + print_var_decl(var, &state);
>>>> }
>>>>
>>>> foreach_list_typed(nir_variable, var, node, &shader->inputs) {
>>>> - print_var_decl(var, &state, fp);
>>>> + print_var_decl(var, &state);
>>>> }
>>>>
>>>> foreach_list_typed(nir_variable, var, node, &shader->outputs) {
>>>> - print_var_decl(var, &state, fp);
>>>> + print_var_decl(var, &state);
>>>> }
>>>>
>>>> foreach_list_typed(nir_variable, var, node, &shader->globals) {
>>>> - print_var_decl(var, &state, fp);
>>>> + print_var_decl(var, &state);
>>>> }
>>>>
>>>> foreach_list_typed(nir_variable, var, node, &shader->system_values) {
>>>> - print_var_decl(var, &state, fp);
>>>> + print_var_decl(var, &state);
>>>> }
>>>>
>>>> foreach_list_typed(nir_register, reg, node, &shader->registers) {
>>>> @@ -887,7 +900,7 @@ nir_print_shader(nir_shader *shader, FILE *fp)
>>>> }
>>>>
>>>> foreach_list_typed(nir_function, func, node, &shader->functions) {
>>>> - print_function(func, &state, fp);
>>>> + print_function(func, &state);
>>>> }
>>>>
>>>> destroy_print_state(&state);
>>>> --
>>>> 2.4.3
>>>>
More information about the mesa-dev
mailing list