I didn't look through it close enough to call it a review, but I like it.
Especially getting rid of src/def_init.
Acked-by: Jason Ekstrand <jason.ekstr...@intel.com>
On Wed, Feb 11, 2015 at 4:32 PM, Eric Anholt <e...@anholt.net> wrote:
> We were filling out almost all fields of almost all instructions, but
> leaving out a couple of them. This simplifies the source code, cuts 700
> bytes from the compiled binary, and prevents developer surprise when one
> field of your otherwise-containing-defaults struct is actually
> uninitialized.
> ---
> src/glsl/nir/nir.c | 138
> +++++++++++------------------------------------------
> 1 file changed, 29 insertions(+), 109 deletions(-)
>
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index b46fd30..1a95060 100644
> --- a/src/glsl/nir/nir.c
> +++ b/src/glsl/nir/nir.c
> @@ -31,7 +31,7 @@
> nir_shader *
> nir_shader_create(void *mem_ctx)
> {
> - nir_shader *shader = ralloc(mem_ctx, nir_shader);
> + nir_shader *shader = rzalloc(mem_ctx, nir_shader);
>
> shader->uniforms = _mesa_hash_table_create(shader,
> _mesa_key_hash_string,
> _mesa_key_string_equal);
> @@ -40,18 +40,10 @@ nir_shader_create(void *mem_ctx)
> shader->outputs = _mesa_hash_table_create(shader,
> _mesa_key_hash_string,
> _mesa_key_string_equal);
>
> - shader->num_user_structures = 0;
> - shader->user_structures = NULL;
> -
> exec_list_make_empty(&shader->functions);
> exec_list_make_empty(&shader->registers);
> exec_list_make_empty(&shader->globals);
> exec_list_make_empty(&shader->system_values);
> - shader->reg_alloc = 0;
> -
> - shader->num_inputs = 0;
> - shader->num_outputs = 0;
> - shader->num_uniforms = 0;
>
> return shader;
> }
> @@ -59,7 +51,7 @@ nir_shader_create(void *mem_ctx)
> static nir_register *
> reg_create(void *mem_ctx, struct exec_list *list)
> {
> - nir_register *reg = ralloc(mem_ctx, nir_register);
> + nir_register *reg = rzalloc(mem_ctx, nir_register);
>
> reg->uses = _mesa_set_create(mem_ctx, _mesa_hash_pointer,
> _mesa_key_pointer_equal);
> @@ -68,11 +60,6 @@ reg_create(void *mem_ctx, struct exec_list *list)
> reg->if_uses = _mesa_set_create(mem_ctx, _mesa_hash_pointer,
> _mesa_key_pointer_equal);
>
> - reg->num_components = 0;
> - reg->num_array_elems = 0;
> - reg->is_packed = false;
> - reg->name = NULL;
> -
> exec_list_push_tail(list, ®->node);
>
> return reg;
> @@ -107,7 +94,7 @@ nir_reg_remove(nir_register *reg)
> nir_function *
> nir_function_create(nir_shader *shader, const char *name)
> {
> - nir_function *func = ralloc(shader, nir_function);
> + nir_function *func = rzalloc(shader, nir_function);
>
> exec_list_push_tail(&shader->functions, &func->node);
> exec_list_make_empty(&func->overload_list);
> @@ -122,12 +109,9 @@ nir_function_overload_create(nir_function *func)
> {
> void *mem_ctx = ralloc_parent(func);
>
> - nir_function_overload *overload = ralloc(mem_ctx,
> nir_function_overload);
> + nir_function_overload *overload = rzalloc(mem_ctx,
> nir_function_overload);
>
> - overload->num_params = 0;
> - overload->params = NULL;
> overload->return_type = glsl_void_type();
> - overload->impl = NULL;
>
> exec_list_push_tail(&func->overload_list, &overload->node);
> overload->function = func;
> @@ -247,7 +231,7 @@ nir_function_impl_create(nir_function_overload
> *overload)
>
> void *mem_ctx = ralloc_parent(overload);
>
> - nir_function_impl *impl = ralloc(mem_ctx, nir_function_impl);
> + nir_function_impl *impl = rzalloc(mem_ctx, nir_function_impl);
>
> overload->impl = impl;
> impl->overload = overload;
> @@ -257,11 +241,6 @@ nir_function_impl_create(nir_function_overload
> *overload)
> exec_list_make_empty(&impl->body);
> exec_list_make_empty(&impl->registers);
> exec_list_make_empty(&impl->locals);
> - impl->num_params = 0;
> - impl->params = NULL;
> - impl->return_var = NULL;
> - impl->reg_alloc = 0;
> - impl->ssa_alloc = 0;
> impl->valid_metadata = nir_metadata_none;
>
> /* create start & end blocks */
> @@ -283,14 +262,12 @@ nir_function_impl_create(nir_function_overload
> *overload)
> nir_block *
> nir_block_create(void *mem_ctx)
> {
> - nir_block *block = ralloc(mem_ctx, nir_block);
> + nir_block *block = rzalloc(mem_ctx, nir_block);
>
> cf_init(&block->cf_node, nir_cf_node_block);
>
> - block->successors[0] = block->successors[1] = NULL;
> block->predecessors = _mesa_set_create(mem_ctx, _mesa_hash_pointer,
> _mesa_key_pointer_equal);
> - block->imm_dom = NULL;
> block->dom_frontier = _mesa_set_create(mem_ctx, _mesa_hash_pointer,
> _mesa_key_pointer_equal);
>
> @@ -299,22 +276,12 @@ nir_block_create(void *mem_ctx)
> return block;
> }
>
> -static inline void
> -src_init(nir_src *src)
> -{
> - src->is_ssa = false;
> - src->reg.reg = NULL;
> - src->reg.indirect = NULL;
> - src->reg.base_offset = 0;
> -}
> -
> nir_if *
> nir_if_create(void *mem_ctx)
> {
> - nir_if *if_stmt = ralloc(mem_ctx, nir_if);
> + nir_if *if_stmt = rzalloc(mem_ctx, nir_if);
>
> cf_init(&if_stmt->cf_node, nir_cf_node_if);
> - src_init(&if_stmt->condition);
>
> nir_block *then = nir_block_create(mem_ctx);
> exec_list_make_empty(&if_stmt->then_list);
> @@ -332,7 +299,7 @@ nir_if_create(void *mem_ctx)
> nir_loop *
> nir_loop_create(void *mem_ctx)
> {
> - nir_loop *loop = ralloc(mem_ctx, nir_loop);
> + nir_loop *loop = rzalloc(mem_ctx, nir_loop);
>
> cf_init(&loop->cf_node, nir_cf_node_loop);
>
> @@ -351,59 +318,33 @@ static void
> instr_init(nir_instr *instr, nir_instr_type type)
> {
> instr->type = type;
> - instr->block = NULL;
> exec_node_init(&instr->node);
> }
>
> -static void
> -dest_init(nir_dest *dest)
> -{
> - dest->is_ssa = false;
> - dest->reg.reg = NULL;
> - dest->reg.indirect = NULL;
> - dest->reg.base_offset = 0;
> -}
> -
> -static void
> -alu_dest_init(nir_alu_dest *dest)
> -{
> - dest_init(&dest->dest);
> - dest->saturate = false;
> - dest->write_mask = 0xf;
> -}
> -
> -static void
> -alu_src_init(nir_alu_src *src)
> -{
> - src_init(&src->src);
> - src->abs = src->negate = false;
> - src->swizzle[0] = 0;
> - src->swizzle[1] = 1;
> - src->swizzle[2] = 2;
> - src->swizzle[3] = 3;
> -}
> -
> nir_alu_instr *
> nir_alu_instr_create(void *mem_ctx, nir_op op)
> {
> unsigned num_srcs = nir_op_infos[op].num_inputs;
> nir_alu_instr *instr =
> - ralloc_size(mem_ctx,
> - sizeof(nir_alu_instr) + num_srcs * sizeof(nir_alu_src));
> + rzalloc_size(mem_ctx,
> + sizeof(nir_alu_instr) + num_srcs *
> sizeof(nir_alu_src));
>
> instr_init(&instr->instr, nir_instr_type_alu);
> instr->op = op;
> - alu_dest_init(&instr->dest);
> - for (unsigned i = 0; i < num_srcs; i++)
> - alu_src_init(&instr->src[i]);
> -
> + instr->dest.write_mask = 0xf;
> + for (unsigned i = 0; i < num_srcs; i++) {
> + instr->src[i].swizzle[0] = 0;
> + instr->src[i].swizzle[1] = 1;
> + instr->src[i].swizzle[2] = 2;
> + instr->src[i].swizzle[3] = 3;
> + }
> return instr;
> }
>
> nir_jump_instr *
> nir_jump_instr_create(void *mem_ctx, nir_jump_type type)
> {
> - nir_jump_instr *instr = ralloc(mem_ctx, nir_jump_instr);
> + nir_jump_instr *instr = rzalloc(mem_ctx, nir_jump_instr);
> instr_init(&instr->instr, nir_instr_type_jump);
> instr->type = type;
> return instr;
> @@ -412,7 +353,7 @@ nir_jump_instr_create(void *mem_ctx, nir_jump_type
> type)
> nir_load_const_instr *
> nir_load_const_instr_create(void *mem_ctx, unsigned num_components)
> {
> - nir_load_const_instr *instr = ralloc(mem_ctx, nir_load_const_instr);
> + nir_load_const_instr *instr = rzalloc(mem_ctx, nir_load_const_instr);
> instr_init(&instr->instr, nir_instr_type_load_const);
>
> nir_ssa_def_init(&instr->instr, &instr->def, num_components, NULL);
> @@ -425,31 +366,24 @@ nir_intrinsic_instr_create(void *mem_ctx,
> nir_intrinsic_op op)
> {
> unsigned num_srcs = nir_intrinsic_infos[op].num_srcs;
> nir_intrinsic_instr *instr =
> - ralloc_size(mem_ctx,
> - sizeof(nir_intrinsic_instr) + num_srcs *
> sizeof(nir_src));
> + rzalloc_size(mem_ctx,
> + sizeof(nir_intrinsic_instr) + num_srcs *
> sizeof(nir_src));
>
> instr_init(&instr->instr, nir_instr_type_intrinsic);
> instr->intrinsic = op;
>
> - if (nir_intrinsic_infos[op].has_dest)
> - dest_init(&instr->dest);
> -
> - for (unsigned i = 0; i < num_srcs; i++)
> - src_init(&instr->src[i]);
> -
> return instr;
> }
>
> nir_call_instr *
> nir_call_instr_create(void *mem_ctx, nir_function_overload *callee)
> {
> - nir_call_instr *instr = ralloc(mem_ctx, nir_call_instr);
> + nir_call_instr *instr = rzalloc(mem_ctx, nir_call_instr);
> instr_init(&instr->instr, nir_instr_type_call);
>
> instr->callee = callee;
> instr->num_params = callee->num_params;
> instr->params = ralloc_array(mem_ctx, nir_deref_var *,
> instr->num_params);
> - instr->return_deref = NULL;
>
> return instr;
> }
> @@ -457,19 +391,11 @@ nir_call_instr_create(void *mem_ctx,
> nir_function_overload *callee)
> nir_tex_instr *
> nir_tex_instr_create(void *mem_ctx, unsigned num_srcs)
> {
> - nir_tex_instr *instr = ralloc(mem_ctx, nir_tex_instr);
> + nir_tex_instr *instr = rzalloc(mem_ctx, nir_tex_instr);
> instr_init(&instr->instr, nir_instr_type_tex);
>
> - dest_init(&instr->dest);
> -
> instr->num_srcs = num_srcs;
> instr->src = ralloc_array(mem_ctx, nir_tex_src, num_srcs);
> - for (unsigned i = 0; i < num_srcs; i++)
> - src_init(&instr->src[i].src);
> -
> - instr->sampler_index = 0;
> - instr->sampler_array_size = 0;
> - instr->sampler = NULL;
>
> return instr;
> }
> @@ -477,10 +403,9 @@ nir_tex_instr_create(void *mem_ctx, unsigned num_srcs)
> nir_phi_instr *
> nir_phi_instr_create(void *mem_ctx)
> {
> - nir_phi_instr *instr = ralloc(mem_ctx, nir_phi_instr);
> + nir_phi_instr *instr = rzalloc(mem_ctx, nir_phi_instr);
> instr_init(&instr->instr, nir_instr_type_phi);
>
> - dest_init(&instr->dest);
> exec_list_make_empty(&instr->srcs);
> return instr;
> }
> @@ -488,7 +413,7 @@ nir_phi_instr_create(void *mem_ctx)
> nir_parallel_copy_instr *
> nir_parallel_copy_instr_create(void *mem_ctx)
> {
> - nir_parallel_copy_instr *instr = ralloc(mem_ctx,
> nir_parallel_copy_instr);
> + nir_parallel_copy_instr *instr = rzalloc(mem_ctx,
> nir_parallel_copy_instr);
> instr_init(&instr->instr, nir_instr_type_parallel_copy);
>
> exec_list_make_empty(&instr->entries);
> @@ -499,7 +424,7 @@ nir_parallel_copy_instr_create(void *mem_ctx)
> nir_ssa_undef_instr *
> nir_ssa_undef_instr_create(void *mem_ctx, unsigned num_components)
> {
> - nir_ssa_undef_instr *instr = ralloc(mem_ctx, nir_ssa_undef_instr);
> + nir_ssa_undef_instr *instr = rzalloc(mem_ctx, nir_ssa_undef_instr);
> instr_init(&instr->instr, nir_instr_type_ssa_undef);
>
> nir_ssa_def_init(&instr->instr, &instr->def, num_components, NULL);
> @@ -510,9 +435,8 @@ nir_ssa_undef_instr_create(void *mem_ctx, unsigned
> num_components)
> nir_deref_var *
> nir_deref_var_create(void *mem_ctx, nir_variable *var)
> {
> - nir_deref_var *deref = ralloc(mem_ctx, nir_deref_var);
> + nir_deref_var *deref = rzalloc(mem_ctx, nir_deref_var);
> deref->deref.deref_type = nir_deref_type_var;
> - deref->deref.child = NULL;
> deref->deref.type = var->type;
> deref->var = var;
> return deref;
> @@ -521,21 +445,17 @@ nir_deref_var_create(void *mem_ctx, nir_variable
> *var)
> nir_deref_array *
> nir_deref_array_create(void *mem_ctx)
> {
> - nir_deref_array *deref = ralloc(mem_ctx, nir_deref_array);
> + nir_deref_array *deref = rzalloc(mem_ctx, nir_deref_array);
> deref->deref.deref_type = nir_deref_type_array;
> - deref->deref.child = NULL;
> deref->deref_array_type = nir_deref_array_type_direct;
> - src_init(&deref->indirect);
> - deref->base_offset = 0;
> return deref;
> }
>
> nir_deref_struct *
> nir_deref_struct_create(void *mem_ctx, unsigned field_index)
> {
> - nir_deref_struct *deref = ralloc(mem_ctx, nir_deref_struct);
> + nir_deref_struct *deref = rzalloc(mem_ctx, nir_deref_struct);
> deref->deref.deref_type = nir_deref_type_struct;
> - deref->deref.child = NULL;
> deref->index = field_index;
> return deref;
> }
> --
> 2.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
