On Fri, May 3, 2019 at 3:29 PM Connor Abbott <[email protected]> wrote:
> FWIW, the reason I changed it away was to keep it consistent with the line > directly above that uses the length (since the C array won't exist if it's > length 0). Does that convince you? > Nope. First off, if you take Dylan's suggestions (which I think are reasonable), it doesn't use the length. Second, it means that the C code will now have an unverifiable random number in it. Are you sure it's really 137? I dare you to go count them. --Jason > On Fri, May 3, 2019 at 10:26 PM Jason Ekstrand <[email protected]> > wrote: > >> On Thu, May 2, 2019 at 3:51 PM Dylan Baker <[email protected]> wrote: >> >>> Quoting Connor Abbott (2019-05-02 13:34:07) >>> > Just don't emit the transform array at all if there are no transforms >>> > for a state, and avoid trying to walk over it. >>> > --- >>> > Brian, does this build on Windows? I tested it on my shader-db >>> > on radeonsi. >>> > >>> > --- >>> > src/compiler/nir/nir_algebraic.py | 6 +++++- >>> > 1 file changed, 5 insertions(+), 1 deletion(-) >>> > >>> > diff --git a/src/compiler/nir/nir_algebraic.py >>> b/src/compiler/nir/nir_algebraic.py >>> > index 6db749e9248..7af80a4f92e 100644 >>> > --- a/src/compiler/nir/nir_algebraic.py >>> > +++ b/src/compiler/nir/nir_algebraic.py >>> > @@ -993,11 +993,13 @@ static const uint16_t CONST_STATE = 1; >>> > % endfor >>> > >>> > % for state_id, state_xforms in enumerate(automaton.state_patterns): >>> > +% if len(state_xforms) > 0: # avoid emitting a 0-length array for MSVC >>> >>> if not state_xforms: >>> >>> Using len() to test container emptiness is an anti-pattern in python, is >>> is >>> roughly 10x slower than this. >>> >>> > static const struct transform ${pass_name}_state${state_id}_xforms[] >>> = { >>> > % for i in state_xforms: >>> > { ${xforms[i].search.c_ptr(cache)}, >>> ${xforms[i].replace.c_value_ptr(cache)}, ${xforms[i].condition_index} }, >>> > % endfor >>> > }; >>> > +% endif >>> > % endfor >>> > >>> > static const struct per_op_table >>> ${pass_name}_table[nir_num_search_ops] = { >>> > @@ -1080,7 +1082,8 @@ ${pass_name}_block(nir_builder *build, nir_block >>> *block, >>> > switch (states[alu->dest.dest.ssa.index]) { >>> > % for i in range(len(automaton.state_patterns)): >>> > case ${i}: >>> > - for (unsigned i = 0; i < >>> ARRAY_SIZE(${pass_name}_state${i}_xforms); i++) { >>> >> >> I'd rather keep the ARRAY_SIZE unless we've got a really good reason to >> drop it. With that and Dylan's changes, >> >> Reviewed-by: Jason Ekstrand <[email protected]> >> >> >>> > + % if len(automaton.state_patterns[i]) > 0: >>> >>> same here >>> >>> Dylan >>> >>> > + for (unsigned i = 0; i < >>> ${len(automaton.state_patterns[i])}; i++) { >>> > const struct transform *xform = >>> &${pass_name}_state${i}_xforms[i]; >>> > if (condition_flags[xform->condition_offset] && >>> > nir_replace_instr(build, alu, xform->search, >>> xform->replace)) { >>> > @@ -1088,6 +1091,7 @@ ${pass_name}_block(nir_builder *build, nir_block >>> *block, >>> > break; >>> > } >>> > } >>> > + % endif >>> > break; >>> > % endfor >>> > default: assert(0); >>> > -- >>> > 2.17.2 >>> > >>> > _______________________________________________ >>> > mesa-dev mailing list >>> > [email protected] >>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >>> >>
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
