<div dir="ltr">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 <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Feb 11, 2015 at 4:32 PM, Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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, &reg->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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>