[Mesa-dev] [PATCH] nir/algebraic: Don't emit empty initializers for MSVC

[Jason Ekstrand]

On Fri, May 3, 2019 at 3:29 PM Connor Abbott <cwabbott0 at 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.

--Jason


> On Fri, May 3, 2019 at 10:26 PM Jason Ekstrand <jason at jlekstrand.net>
> wrote:
>
>> On Thu, May 2, 2019 at 3:51 PM Dylan Baker <dylan at 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 <jason at 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 at lists.freedesktop.org
>>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190503/060cf0b8/attachment-0001.html>