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?
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
