[Mesa-dev] [PATCH] nir/algebraic: Don't emit empty initializers for MSVC
Connor Abbott
cwabbott0 at gmail.com
Fri May 3 20:29:36 UTC 2019
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 <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/9a4dbdac/attachment-0001.html>
More information about the mesa-dev
mailing list