<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Oct 22, 2018 at 5:50 PM Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I think a better description is "nir/search: Use nir_imm_* helpers". Using nir_builder is just a prerequisite to doing that. </blockquote><div> </div><div>Good call. The new commit message is:</div><div> </div><div> nir/search: Use the nir_imm_* helpers from nir_builder This requires that we rework the interface a bit to use nir_builder but that's a nice little modernization anyway. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> On 10/22/2018 03:13 PM, Jason Ekstrand wrote: > --- > src/compiler/nir/nir_algebraic.py | 14 ++-- > src/compiler/nir/nir_search.c | 111 ++++++++---------------------- > src/compiler/nir/nir_search.h | 9 ++- > 3 files changed, 43 insertions(+), 91 deletions(-) > > diff --git a/src/compiler/nir/nir_algebraic.py b/src/compiler/nir/nir_algebraic.py > index 4134d496030..d2374d3216a 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -563,6 +563,7 @@ class SearchAndReplace(object): > > _algebraic_pass_template = mako.template.Template(""" > #include "nir.h" > +#include "nir_builder.h" > #include "nir_search.h" > #include "nir_search_helpers.h" > > @@ -591,8 +592,8 @@ static const struct transform ${pass_name}_${opcode}_xforms[] = { > % endfor > > static bool > -${pass_name}_block(nir_block *block, const bool *condition_flags, > - void *mem_ctx) > +${pass_name}_block(nir_builder *build, nir_block *block, > + const bool *condition_flags) > { > bool progress = false; > > @@ -610,8 +611,7 @@ ${pass_name}_block(nir_block *block, const bool *condition_flags, > for (unsigned i = 0; i < ARRAY_SIZE(${pass_name}_${opcode}_xforms); i++) { > const struct transform *xform = &${pass_name}_${opcode}_xforms[i]; > if (condition_flags[xform->condition_offset] && > - nir_replace_instr(alu, xform->search, xform->replace, > - mem_ctx)) { > + nir_replace_instr(build, alu, xform->search, xform->replace)) { > progress = true; > break; > } > @@ -629,11 +629,13 @@ ${pass_name}_block(nir_block *block, const bool *condition_flags, > static bool > ${pass_name}_impl(nir_function_impl *impl, const bool *condition_flags) > { > - void *mem_ctx = ralloc_parent(impl); > bool progress = false; > > + nir_builder build; > + nir_builder_init(&build, impl); > + > nir_foreach_block_reverse(block, impl) { > - progress |= ${pass_name}_block(block, condition_flags, mem_ctx); > + progress |= ${pass_name}_block(&build, block, condition_flags); > } > > if (progress) > diff --git a/src/compiler/nir/nir_search.c b/src/compiler/nir/nir_search.c > index 1686b6bd0de..0270302fd3d 100644 > --- a/src/compiler/nir/nir_search.c > +++ b/src/compiler/nir/nir_search.c > @@ -27,6 +27,7 @@ > > #include <inttypes.h> > #include "nir_search.h" > +#include "nir_builder.h" > #include "util/half_float.h" > > struct match_state { > @@ -408,10 +409,11 @@ bitsize_tree_filter_down(bitsize_tree *tree, unsigned size) > } > > static nir_alu_src > -construct_value(const nir_search_value *value, > +construct_value(nir_builder *build, > + const nir_search_value *value, > unsigned num_components, bitsize_tree *bitsize, > struct match_state *state, > - nir_instr *instr, void *mem_ctx) > + nir_instr *instr) > { > switch (value->type) { > case nir_search_value_expression: { > @@ -420,7 +422,7 @@ construct_value(const nir_search_value *value, > if (nir_op_infos[expr->opcode].output_size != 0) > num_components = nir_op_infos[expr->opcode].output_size; > > - nir_alu_instr *alu = nir_alu_instr_create(mem_ctx, expr->opcode); > + nir_alu_instr *alu = nir_alu_instr_create(build->shader, expr->opcode); > nir_ssa_dest_init(&alu->instr, &alu->dest.dest, num_components, > bitsize->dest_size, NULL); > alu->dest.write_mask = (1 << num_components) - 1; > @@ -440,12 +442,12 @@ construct_value(const nir_search_value *value, > if (nir_op_infos[alu->op].input_sizes[i] != 0) > num_components = nir_op_infos[alu->op].input_sizes[i]; > > - alu->src[i] = construct_value(expr->srcs[i], > + alu->src[i] = construct_value(build, expr->srcs[i], > num_components, bitsize->srcs[i], > - state, instr, mem_ctx); > + state, instr); > } > > - nir_instr_insert_before(instr, &alu->instr); > + nir_builder_instr_insert(build, &alu->instr); > > nir_alu_src val; > val.src = nir_src_for_ssa(&alu->dest.dest.ssa); > @@ -461,8 +463,8 @@ construct_value(const nir_search_value *value, > assert(state->variables_seen & (1 << var->variable)); > > nir_alu_src val = { NIR_SRC_INIT }; > - nir_alu_src_copy(&val, &state->variables[var->variable], mem_ctx); > - > + nir_alu_src_copy(&val, &state->variables[var->variable], > + (void *)build->shader); > assert(!var->is_constant); > > return val; > @@ -470,79 +472,27 @@ construct_value(const nir_search_value *value, > > case nir_search_value_constant: { > const nir_search_constant *c = nir_search_value_as_constant(value); > - nir_load_const_instr *load = > - nir_load_const_instr_create(mem_ctx, 1, bitsize->dest_size); > > + nir_ssa_def *cval; > switch (c->type) { > case nir_type_float: > - load-><a href="http://def.name" rel="noreferrer" target="_blank">def.name</a> = ralloc_asprintf(load, "%f", c->data.d); > - switch (bitsize->dest_size) { > - case 16: > - load->value.u16[0] = _mesa_float_to_half(c->data.d); > - break; > - case 32: > - load->value.f32[0] = c->data.d; > - break; > - case 64: > - load->value.f64[0] = c->data.d; > - break; > - default: > - unreachable("unknown bit size"); > - } > + cval = nir_imm_floatN_t(build, c->data.d, bitsize->dest_size); > break; > > case nir_type_int: > - load-><a href="http://def.name" rel="noreferrer" target="_blank">def.name</a> = ralloc_asprintf(load, "%" PRIi64, c->data.i); > - switch (bitsize->dest_size) { > - case 8: > - load->value.i8[0] = c->data.i; > - break; > - case 16: > - load->value.i16[0] = c->data.i; > - break; > - case 32: > - load->value.i32[0] = c->data.i; > - break; > - case 64: > - load->value.i64[0] = c->data.i; > - break; > - default: > - unreachable("unknown bit size"); > - } > - break; > - > case nir_type_uint: > - load-><a href="http://def.name" rel="noreferrer" target="_blank">def.name</a> = ralloc_asprintf(load, "%" PRIu64, c->data.u); > - switch (bitsize->dest_size) { > - case 8: > - load->value.u8[0] = c->data.u; > - break; > - case 16: > - load->value.u16[0] = c->data.u; > - break; > - case 32: > - load->value.u32[0] = c->data.u; > - break; > - case 64: > - load->value.u64[0] = c->data.u; > - break; > - default: > - unreachable("unknown bit size"); > - } > + cval = nir_imm_intN_t(build, c->data.i, bitsize->dest_size); > break; > > case nir_type_bool: > - assert(bitsize->dest_size == 32); > - load->value.u32[0] = c->data.u; > + cval = nir_imm_bool(build, c->data.u); > break; > default: > unreachable("Invalid alu source type"); > } > > - nir_instr_insert_before(instr, &load->instr); > - > nir_alu_src val; > - val.src = nir_src_for_ssa(&load->def); > + val.src = nir_src_for_ssa(cval); > val.negate = false; > val.abs = false, > memset(val.swizzle, 0, sizeof val.swizzle); > @@ -555,9 +505,10 @@ construct_value(const nir_search_value *value, > } > } > > -nir_alu_instr * > -nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search, > - const nir_search_value *replace, void *mem_ctx) > +nir_ssa_def * > +nir_replace_instr(nir_builder *build, nir_alu_instr *instr, > + const nir_search_expression *search, > + const nir_search_value *replace) > { > uint8_t swizzle[NIR_MAX_VEC_COMPONENTS] = { 0 }; > > @@ -580,23 +531,19 @@ nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search, > bitsize_tree_filter_up(tree); > bitsize_tree_filter_down(tree, instr->dest.dest.ssa.bit_size); > > + build->cursor = nir_before_instr(&instr->instr); > + > + nir_alu_src val = construct_value(build, replace, > + instr->dest.dest.ssa.num_components, > + tree, &state, &instr->instr); > + > /* Inserting a mov may be unnecessary. However, it's much easier to > * simply let copy propagation clean this up than to try to go through > * and rewrite swizzles ourselves. > */ > - nir_alu_instr *mov = nir_alu_instr_create(mem_ctx, nir_op_imov); > - mov->dest.write_mask = instr->dest.write_mask; > - nir_ssa_dest_init(&mov->instr, &mov->dest.dest, > - instr->dest.dest.ssa.num_components, > - instr->dest.dest.ssa.bit_size, NULL); > - > - mov->src[0] = construct_value(replace, > - instr->dest.dest.ssa.num_components, tree, > - &state, &instr->instr, mem_ctx); > - nir_instr_insert_before(&instr->instr, &mov->instr); > - > - nir_ssa_def_rewrite_uses(&instr->dest.dest.ssa, > - nir_src_for_ssa(&mov->dest.dest.ssa)); > + nir_ssa_def *ssa_val = > + nir_imov_alu(build, val, instr->dest.dest.ssa.num_components); > + nir_ssa_def_rewrite_uses(&instr->dest.dest.ssa, nir_src_for_ssa(ssa_val)); > > /* We know this one has no more uses because we just rewrote them all, > * so we can remove it. The rest of the matched expression, however, we > @@ -606,5 +553,5 @@ nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search, > > ralloc_free(bitsize_ctx); > > - return mov; > + return ssa_val; > } > diff --git a/src/compiler/nir/nir_search.h b/src/compiler/nir/nir_search.h > index 8b409541532..df4189ede74 100644 > --- a/src/compiler/nir/nir_search.h > +++ b/src/compiler/nir/nir_search.h > @@ -32,6 +32,8 @@ > > #define NIR_SEARCH_MAX_VARIABLES 16 > > +struct nir_builder; > + > typedef enum { > nir_search_value_expression, > nir_search_value_variable, > @@ -123,8 +125,9 @@ NIR_DEFINE_CAST(nir_search_value_as_expression, nir_search_value, > nir_search_expression, value, > type, nir_search_value_expression) > > -nir_alu_instr * > -nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search, > - const nir_search_value *replace, void *mem_ctx); > +nir_ssa_def * > +nir_replace_instr(struct nir_builder *b, nir_alu_instr *instr, > + const nir_search_expression *search, > + const nir_search_value *replace); > > #endif /* _NIR_SEARCH_ */ > </blockquote></div></div></div></div></div>