<div dir="ltr">These two patches provide a slightly different approach to what Tim did. I think Tim's is fine but I also like the idea of conditions working based on SSA def, type, and read mask so I consider this a unification/cleanup rather than a counter-proposal. Thoughts? </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Jan 11, 2017 at 11:13 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Instead of passing all of the ALU op information, we just pass what you need: The SSA def, the type it's being read as, and a component mask. --- src/compiler/nir/nir_search.c | 12 +++++-- src/compiler/nir/nir_search.h | 3 +- src/compiler/nir/nir_search_helpers.h | 62 +++++++++++++++++++---------------- 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/compiler/nir/nir_search.c b/src/compiler/nir/nir_search.c index 10a0941..2f57821 100644 --- a/src/compiler/nir/nir_search.c +++ b/src/compiler/nir/nir_search.c @@ -154,8 +154,16 @@ match_value(const nir_search_value *value, nir_alu_instr *instr, unsigned src, instr->src[src].src.ssa->parent_instr->type != nir_instr_type_load_const) return false; - if (var->cond && !var->cond(instr, src, num_components, new_swizzle)) - return false; + if (var->cond) { + uint8_t read_mask = 0; + for (unsigned i = 0; i < num_components; i++) + read_mask |= 1 << new_swizzle[i]; + + if (!var->cond(instr->src[src].src.ssa, + nir_op_infos[instr->op].input_types[src], + read_mask)) + return false; + } if (var->type != nir_type_invalid && !src_is_type(instr->src[src].src, var->type)) diff --git a/src/compiler/nir/nir_search.h b/src/compiler/nir/nir_search.h index dec19d5..9d25018 100644 --- a/src/compiler/nir/nir_search.h +++ b/src/compiler/nir/nir_search.h @@ -76,8 +76,7 @@ typedef struct { * variables to require, for example, power-of-two in order for the search * to match. */ - bool (*cond)(nir_alu_instr *instr, unsigned src, - unsigned num_components, const uint8_t *swizzle); + bool (*cond)(nir_ssa_def *def, nir_alu_type type, uint8_t read_mask); } nir_search_variable; typedef struct { diff --git a/src/compiler/nir/nir_search_helpers.h b/src/compiler/nir/nir_search_helpers.h index 20fdae6..85f6c85 100644 --- a/src/compiler/nir/nir_search_helpers.h +++ b/src/compiler/nir/nir_search_helpers.h @@ -36,25 +36,26 @@ __is_power_of_two(unsigned int x) } static inline bool -is_pos_power_of_two(nir_alu_instr *instr, unsigned src, unsigned num_components, - const uint8_t *swizzle) +is_pos_power_of_two(nir_ssa_def *def, nir_alu_type type, uint8_t read_mask) { - nir_const_value *val = nir_src_as_const_value(instr->src[src].src); - - /* only constant src's: */ - if (!val) + if (def->parent_instr->type != nir_instr_type_load_const) return false; - for (unsigned i = 0; i < num_components; i++) { - switch (nir_op_infos[instr->op].input_types[src]) { + nir_const_value *val = &nir_instr_as_load_const(def->parent_instr)->value; + + for (unsigned i = 0; i < 4; i++) { + if (!(read_mask & (1 << i))) + continue; + + switch (type) { case nir_type_int: - if (val->i32[swizzle[i]] < 0) + if (val->i32[i] < 0) return false; - if (!__is_power_of_two(val->i32[swizzle[i]])) + if (!__is_power_of_two(val->i32[i])) return false; break; case nir_type_uint: - if (!__is_power_of_two(val->u32[swizzle[i]])) + if (!__is_power_of_two(val->u32[i])) return false; break; default: @@ -66,21 +67,22 @@ is_pos_power_of_two(nir_alu_instr *instr, unsigned src, unsigned num_components, } static inline bool -is_neg_power_of_two(nir_alu_instr *instr, unsigned src, unsigned num_components, - const uint8_t *swizzle) +is_neg_power_of_two(nir_ssa_def *def, nir_alu_type type, uint8_t read_mask) { - nir_const_value *val = nir_src_as_const_value(instr->src[src].src); - - /* only constant src's: */ - if (!val) + if (def->parent_instr->type != nir_instr_type_load_const) return false; - for (unsigned i = 0; i < num_components; i++) { - switch (nir_op_infos[instr->op].input_types[src]) { + nir_const_value *val = &nir_instr_as_load_const(def->parent_instr)->value; + + for (unsigned i = 0; i < 4; i++) { + if (!(read_mask & (1 << i))) + continue; + + switch (type) { case nir_type_int: - if (val->i32[swizzle[i]] > 0) + if (val->i32[i] > 0) return false; - if (!__is_power_of_two(abs(val->i32[swizzle[i]]))) + if (!__is_power_of_two(abs(val->i32[i]))) return false; break; default: @@ -92,18 +94,20 @@ is_neg_power_of_two(nir_alu_instr *instr, unsigned src, unsigned num_components, } static inline bool -is_zero_to_one(nir_alu_instr *instr, unsigned src, unsigned num_components, - const uint8_t *swizzle) +is_zero_to_one(nir_ssa_def *def, nir_alu_type type, uint8_t read_mask) { - nir_const_value *val = nir_src_as_const_value(instr->src[src].src); - - if (!val) + if (def->parent_instr->type != nir_instr_type_load_const) return false; - for (unsigned i = 0; i < num_components; i++) { - switch (nir_op_infos[instr->op].input_types[src]) { + nir_const_value *val = &nir_instr_as_load_const(def->parent_instr)->value; + + for (unsigned i = 0; i < 4; i++) { + if (!(read_mask & (1 << i))) + continue; + + switch (type) { case nir_type_float: - if (val->f32[swizzle[i]] < 0.0f || val->f32[swizzle[i]] > 1.0f) + if (val->f32[i] < 0.0f || val->f32[i] > 1.0f) return false; break; default: -- 2.5.0.400.gff86faf </blockquote></div> </div>