On Fri, May 3, 2019 at 10:39 PM Jason Ekstrand <ja...@jlekstrand.net> wrote:
> On Fri, May 3, 2019 at 3:29 PM Connor Abbott <cwabbo...@gmail.com> 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.
>
Ok, I pushed it with your change.
Connor
>
> --Jason
>
>
>> On Fri, May 3, 2019 at 10:26 PM Jason Ekstrand <ja...@jlekstrand.net>
>> wrote:
>>
>>> On Thu, May 2, 2019 at 3:51 PM Dylan Baker <dy...@pnwbakers.com> 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 <ja...@jlekstrand.net>
>>>
>>>
>>>> > + % 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
>>>> > mesa-dev@lists.freedesktop.org
>>>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
