On Mon, Oct 22, 2018 at 5:50 PM Ian Romanick <i...@freedesktop.org> wrote:
> I think a better description is "nir/search: Use nir_imm_* helpers". > Using nir_builder is just a prerequisite to doing that. > Good call. The new commit message is: 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. > 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->def.name = 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->def.name = 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->def.name = 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_ */ > > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev