<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, May 3, 2019 at 3:29 PM Connor Abbott <<a href="mailto:cwabbott0@gmail.com">cwabbott0@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">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?<br></div></blockquote><div><br></div><div>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.</div><div><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, May 3, 2019 at 10:26 PM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, May 2, 2019 at 3:51 PM Dylan Baker <<a href="mailto:dylan@pnwbakers.com" target="_blank">dylan@pnwbakers.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Connor Abbott (2019-05-02 13:34:07)<br>
> Just don't emit the transform array at all if there are no transforms<br>
> for a state, and avoid trying to walk over it.<br>
> ---<br>
> Brian, does this build on Windows? I tested it on my shader-db<br>
> on radeonsi.<br>
> <br>
> ---<br>
>  src/compiler/nir/nir_algebraic.py | 6 +++++-<br>
>  1 file changed, 5 insertions(+), 1 deletion(-)<br>
> <br>
> diff --git a/src/compiler/nir/nir_algebraic.py b/src/compiler/nir/nir_algebraic.py<br>
> index 6db749e9248..7af80a4f92e 100644<br>
> --- a/src/compiler/nir/nir_algebraic.py<br>
> +++ b/src/compiler/nir/nir_algebraic.py<br>
> @@ -993,11 +993,13 @@ static const uint16_t CONST_STATE = 1;<br>
>  % endfor<br>
>  <br>
>  % for state_id, state_xforms in enumerate(automaton.state_patterns):<br>
> +% if len(state_xforms) > 0: # avoid emitting a 0-length array for MSVC<br>
<br>
if not state_xforms:<br>
<br>
Using len() to test container emptiness is an anti-pattern in python, is is<br>
roughly 10x slower  than this.<br>
<br>
>  static const struct transform ${pass_name}_state${state_id}_xforms[] = {<br>
>  % for i in state_xforms:<br>
>    { ${xforms[i].search.c_ptr(cache)}, ${xforms[i].replace.c_value_ptr(cache)}, ${xforms[i].condition_index} },<br>
>  % endfor<br>
>  };<br>
> +% endif<br>
>  % endfor<br>
>  <br>
>  static const struct per_op_table ${pass_name}_table[nir_num_search_ops] = {<br>
> @@ -1080,7 +1082,8 @@ ${pass_name}_block(nir_builder *build, nir_block *block,<br>
>        switch (states[alu->dest.dest.ssa.index]) {<br>
>  % for i in range(len(automaton.state_patterns)):<br>
>        case ${i}:<br>
> -         for (unsigned i = 0; i < ARRAY_SIZE(${pass_name}_state${i}_xforms); i++) {<br></blockquote><div><br></div><div>I'd rather keep the ARRAY_SIZE unless we've got a really good reason to drop it.  With that and Dylan's changes,</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +         % if len(automaton.state_patterns[i]) > 0:<br>
<br>
same here<br>
<br>
Dylan<br>
<br>
> +         for (unsigned i = 0; i < ${len(automaton.state_patterns[i])}; i++) {<br>
>              const struct transform *xform = &${pass_name}_state${i}_xforms[i];<br>
>              if (condition_flags[xform->condition_offset] &&<br>
>                  nir_replace_instr(build, alu, xform->search, xform->replace)) {<br>
> @@ -1088,6 +1091,7 @@ ${pass_name}_block(nir_builder *build, nir_block *block,<br>
>                 break;<br>
>              }<br>
>           }<br>
> +         % endif<br>
>           break;<br>
>  % endfor<br>
>        default: assert(0);<br>
> -- <br>
> 2.17.2<br>
> <br>
> _______________________________________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div>
</blockquote></div>
</blockquote></div></div>