On Tue, May 10, 2016 at 11:57 AM, Rob Clark <robdcl...@gmail.com> wrote:
> From: Rob Clark <robcl...@freedesktop.org> > > Some optimizations, like converting integer multiply/divide into left/ > right shifts, have additional constraints on the search expression. > Like requiring that a variable is a constant power of two. Support > these cases by allowing a fxn name to be appended to the search var > expression (ie. "a#32(is_power_of_two)"). > > TODO update doc/comment explaining search var syntax > TODO the eagle-eyed viewer might have noticed that this could also > replace the existing const syntax (ie. "#a"). Not sure if we should > keep that.. we could make it syntactic sugar (ie '#' automatically sets > the cond fxn ptr to 'is_const') or just get rid of it entirely? Maybe > that is a follow-on clean-up patch? > > Signed-off-by: Rob Clark <robcl...@freedesktop.org> > --- > src/compiler/nir/nir_algebraic.py | 8 +++-- > src/compiler/nir/nir_opt_algebraic.py | 5 +++ > src/compiler/nir/nir_search.c | 3 ++ > src/compiler/nir/nir_search.h | 10 ++++++ > src/compiler/nir/nir_search_helpers.h | 66 > +++++++++++++++++++++++++++++++++++ > 5 files changed, 90 insertions(+), 2 deletions(-) > create mode 100644 src/compiler/nir/nir_search_helpers.h > > diff --git a/src/compiler/nir/nir_algebraic.py > b/src/compiler/nir/nir_algebraic.py > index 285f853..19ac6ee 100644 > --- a/src/compiler/nir/nir_algebraic.py > +++ b/src/compiler/nir/nir_algebraic.py > @@ -76,6 +76,7 @@ class Value(object): > return Constant(val, name_base) > > __template = mako.template.Template(""" > +#include "compiler/nir/nir_search_helpers.h" > static const ${val.c_type} ${val.name} = { > { ${val.type_enum}, ${val.bit_size} }, > % if isinstance(val, Constant): > @@ -84,6 +85,7 @@ static const ${val.c_type} ${val.name} = { > ${val.index}, /* ${val.var_name} */ > ${'true' if val.is_constant else 'false'}, > ${val.type() or 'nir_type_invalid' }, > + ${val.cond if val.cond else 'NULL'}, > % elif isinstance(val, Expression): > ${'true' if val.inexact else 'false'}, > nir_op_${val.opcode}, > @@ -113,7 +115,7 @@ static const ${val.c_type} ${val.name} = { > Variable=Variable, > Expression=Expression) > > -_constant_re = re.compile(r"(?P<value>[^@]+)(?:@(?P<bits>\d+))?") > +_constant_re = re.compile(r"(?P<value>[^@\(]+)(?:@(?P<bits>\d+))?") > Spurious change? > > class Constant(Value): > def __init__(self, val, name): > @@ -150,7 +152,8 @@ class Constant(Value): > return "nir_type_float" > > _var_name_re = re.compile(r"(?P<const>#)?(?P<name>\w+)" > - > r"(?:@(?P<type>int|uint|bool|float)?(?P<bits>\d+)?)?") > + > r"(?:@(?P<type>int|uint|bool|float)?(?P<bits>\d+)?)?" > + r"(?P<cond>\([^\)]+\))?") > > class Variable(Value): > def __init__(self, val, name, varset): > @@ -161,6 +164,7 @@ class Variable(Value): > > self.var_name = m.group('name') > self.is_constant = m.group('const') is not None > + self.cond = m.group('cond') > self.required_type = m.group('type') > self.bit_size = int(m.group('bits')) if m.group('bits') else 0 > > diff --git a/src/compiler/nir/nir_opt_algebraic.py > b/src/compiler/nir/nir_opt_algebraic.py > index 0a95725..952a91a 100644 > --- a/src/compiler/nir/nir_opt_algebraic.py > +++ b/src/compiler/nir/nir_opt_algebraic.py > @@ -62,6 +62,11 @@ d = 'd' > # constructed value should have that bit-size. > > optimizations = [ > + > + (('imul', a, '#b@32(is_power_of_two)'), ('ishl', a, ('find_lsb', b))), > + (('udiv', a, '#b@32(is_power_of_two)'), ('ushr', a, ('find_lsb', b))), > + (('umod', a, '#b(is_power_of_two)'), ('iand', a, ('isub', b, 1))), > + > (('fneg', ('fneg', a)), a), > (('ineg', ('ineg', a)), a), > (('fabs', ('fabs', a)), ('fabs', a)), > diff --git a/src/compiler/nir/nir_search.c b/src/compiler/nir/nir_search.c > index 2c2fd92..b21fb2c 100644 > --- a/src/compiler/nir/nir_search.c > +++ b/src/compiler/nir/nir_search.c > @@ -127,6 +127,9 @@ 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->type != nir_type_invalid) { > if (instr->src[src].src.ssa->parent_instr->type != > nir_instr_type_alu) > return false; > diff --git a/src/compiler/nir/nir_search.h b/src/compiler/nir/nir_search.h > index a500feb..f55d797 100644 > --- a/src/compiler/nir/nir_search.h > +++ b/src/compiler/nir/nir_search.h > @@ -68,6 +68,16 @@ typedef struct { > * never match anything. > */ > nir_alu_type type; > + > + /** Optional condition fxn ptr > + * > + * This is only allowed in search expressions, and allows additional > + * constraints to be placed on the match. Typically used for > 'is_constant' > + * 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); > } nir_search_variable; > > typedef struct { > diff --git a/src/compiler/nir/nir_search_helpers.h > b/src/compiler/nir/nir_search_helpers.h > new file mode 100644 > index 0000000..8ed2ca0 > --- /dev/null > +++ b/src/compiler/nir/nir_search_helpers.h > @@ -0,0 +1,66 @@ > +/* > + * Copyright © 2016 Red Hat > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > "Software"), > + * to deal in the Software without restriction, including without > limitation > + * the rights to use, copy, modify, merge, publish, distribute, > sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the > next > + * paragraph) shall be included in all copies or substantial portions of > the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT > SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Rob Clark <robcl...@freedesktop.org> > + */ > + > +#ifndef _NIR_SEARCH_HELPERS_ > +#define _NIR_SEARCH_HELPERS_ > + > +#include "nir.h" > + > +static inline bool > +__is_power_of_two(unsigned int x) > +{ > + return ((x != 0) && !(x & (x - 1))); > +} > + > +static inline bool > +is_power_of_two(nir_alu_instr *instr, unsigned src, unsigned > num_components, > + const uint8_t *swizzle) > +{ > + nir_const_value *val = nir_src_as_const_value(instr->src[src].src); > + > + /* only constant src's: */ > + if (!val) > + return false; > + > + for (unsigned i = 0; i < num_components; i++) { > + switch (nir_op_infos[instr->op].input_types[src]) { > + case nir_type_int: > + if (!__is_power_of_two(val->i32[swizzle[i]])) > + return false; > + break; > + case nir_type_uint: > + if (!__is_power_of_two(val->u32[swizzle[i]])) > + return false; > Your implementation of __is_power_of_two takes an unsigned. There's no benefit to splitting it into two cases and it just creates a false distinction. If you do, you can probably inline the helper (I don't care much one way or the other on that). All in all, this seems ok. The "(some_function)" syntax seems clunky but I can't come up with anything better off-hand. Let's just go with it for now and we can always change it later. Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net> > + break; > + default: > + return false; > + } > + } > + > + return true; > +} > + > +#endif /* _NIR_SEARCH_ */ > -- > 2.5.5 > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev