Quoting Ian Romanick (2017-11-13 13:34:15)
> On 11/13/2017 10:49 AM, Dylan Baker wrote:
> > Quoting Ian Romanick (2017-11-10 14:32:49)
> > [snip]
> >
> >> +
> >> +def collect_data(spirv):
> >> + for x in spirv["operand_kinds"]:
> >> + if x["kind"] == "Capability":
> >> + operands = x
> >
> > This makes me nervous. dict iteration order is not guaranteed to be
> > repeatable
> > in python. I think you should either use sorted() [for x in
> > sorted(spirv...)],
> > or if there's only every going to be on case where x[kind] == capability
> > use a
> > break statement and leave a comment.
> >
> >> +
> >> + # There are some duplicate values in some of the tables (thanks
> >> guys!), so
> >> + # filter them out.
> >> +
> >> + # by_value is a dictionary that maps values of enumerants to tuples of
> >> + # enumerant names and capabilities.
> >> + by_value = {}
> >> +
> >> + # by_name is a dictionary that maps names of enumerants to tuples of
> >> + # values and required capabilities.
> >> + by_name = {}
> >> +
> >> + for x in operands["enumerants"]:
> >> + caps = x["capabilities"] if "capabilities" in x else []
> >
> > caps = x.get("capabilities", [])
>
> Yeah... there are a lot of places where I can use that.
>
> >
> >> +
> >> + if x["value"] not in by_value:
> >> + by_value[x["value"]] = (x["enumerant"], caps)
> >> +
> >> + by_name[x["enumerant"]] = (x["value"], caps)
> >> +
> >> + # Recall that there are some duplicate values in the table. These
> >> + # duplicate values also appear in the "capabilities" list for some
> >> + # enumerants. Filter out the duplicates there as well.
> >> + for capability in by_name:
> >> + cap_value, dependencies = by_name[capability]
> >> + for dependency in dependencies:
> >> + dep_value, skip = by_name[dependency]
> >> + real_dependency, skip = by_value[dep_value]
> >
> > I think you can simplify this somewhat:
> >
> > for capability, (_, dependencies) in by_name.iteritems():
> > for dep_value, _ in dependencies.itervalues():
> > real_dependency, _ = by_value[dep_value]
>
> Ah! _ is the idiom I was trying to remember.
>
> >> +
> >> + # In most cases where there is a duplicate capability, things
> >> that
> >> + # depend on one will also depend on the others.
> >> + # StorageBuffer16BitAccess and StorageUniformBufferBlock16
> >> have
> >> + # the same value, and StorageUniform16 depends on both.
> >> + #
> >> + # There are exceptions. ShaderViewportIndexLayerEXT and
> >> + # ShaderViewportIndexLayerNV have the same value, but
> >> + # ShaderViewportMaskNV only depends on
> >> ShaderViewportIndexLayerNV.
> >> + #
> >> + # That's the only case so far, so emit a warning for other
> >> cases
> >> + # that have more than one dependency. That way we can double
> >> + # check that they are handled correctly.
> >> +
> >> + if real_dependency != dependency:
> >> + if real_dependency not in by_name[capability][1]:
> >> + if len(by_name[capability][1]) > 1:
> >> + print("Warning! Removed {} from {}, but no name
> >> with the same value is in the dependency list.".format(dependency,
> >> capability))
> >> + else:
> >> + if len(by_name[capability][1]) == 1:
> >> + print("Error! Cannot remove {} from {} because
> >> it is the only dependency.".format(dependency, capability))
> >
> > I think you want to add file=sys.stderr to the print command. (You might
> > need to
> > import sys, I snipped that part of the patch already...)
> >
> >> + exit(1)
> >> +
> >> + by_name[capability][1].remove(dependency)
> >> +
> >> + # The table generated from this data and the C code that uses it
> >> + # assumes that each capability has a single dependency. That is
> >> + # currently the case, but it may change in the future.
> >> + if len(by_name[capability][1]) > 1:
> >> + print("Error! Too many dependencies left for {}.
> >> {}".format(capability, by_name[capability][1]))
> >> + exit(1)
> >> +
> >> + for cap_value in by_value:
> >> + name, skip = by_value[cap_value]
> >> + by_value[cap_value] = (name, by_name[name][1])
> >> +
> >> + return (by_name, by_value)
> >> +
> >> +TEMPLATE_H = Template(COPYRIGHT + """\
> >> +#ifndef SPIRV_CAPABILITIES_H
> >> +#define SPIRV_CAPABILITIES_H
> >> +
> >> +#include <stdint.h>
> >> +#include "spirv.h"
> >> +#include "util/bitset.h"
> >> +#ifndef ARRAY_SIZE
> >> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
> >> +#endif
> >> +
> >> +#define NO_DEPENDENCY ((${"uint16_t" if len(all_values) > 256 else
> >> "uint8_t"}) ~0)
> >> +
> >> +class spirv_capability_set;
> >> +
> >> +/**
> >> + * Iterator for the enabled capabilities in a spirv_capability_set
> >> + *
> >> + * Roughly, this is a wrapper for the bitset iterator functions.
> >> Dereferencing
> >> + * the iterator results in the \c SpvCapability where as the bitset
> >> iterator
> >> + * functions provide the index in the bitset. The mapping is handled by
> >> + * \c spirv_capability_set.
> >> + */
> >> +class spirv_capability_set_iterator {
> >> +public:
> >> + spirv_capability_set_iterator(const spirv_capability_set *_s);
> >> + inline bool operator==(const spirv_capability_set_iterator &that)
> >> const;
> >> + inline bool operator!=(const spirv_capability_set_iterator &that)
> >> const;
> >> + spirv_capability_set_iterator &operator++();
> >> + SpvCapability operator*() const;
> >> +
> >> +private:
> >> + /** Capability set being iterated. */
> >> + const spirv_capability_set *s;
> >> +
> >> + /** Current index in the set. */
> >> + unsigned i;
> >> +
> >> + /** Temporary storage used by \c __bitset_next_set */
> >> + BITSET_WORD tmp;
> >> +
> >> + /**
> >> + * Construct an iterator starting a specific index
> >> + *
> >> + * Used by \c spirv_capability_set::end().
> >> + */
> >> + spirv_capability_set_iterator(const spirv_capability_set *_s, unsigned
> >> _i);
> >> +
> >> + friend class spirv_capability_set;
> >> +};
> >> +
> >> +/**
> >> + * Set of SPIR-V capabilities
> >> + *
> >> + * All SPIR-V capability values are mapped to a compacted bitset. The
> >> mapping
> >> + * of implemented in the (private) methods \c ::capability_as_index() and
> >> + * \c ::index_as_capability(). Once all of the capabilities for a
> >> particular
> >> + * SPIR-V program have been enabled (by repeatedly calling \c
> >> ::enable()), the
> >> + * set can be reduced to the minimum set by \c ::reduce().
> >> + */
> >> +class spirv_capability_set {
> >> +public:
> >> + /**
> >> + * Construct a capability set with no capabilities enabled.
> >> + */
> >> + spirv_capability_set()
> >> + {
> >> + BITSET_ZERO(capabilities);
> >> + }
> >> +
> >> + /**
> >> + * Enable the specified SPIR-V capability
> >> + */
> >> + inline void enable(SpvCapability cap)
> >> + {
> >> + BITSET_SET(capabilities, capability_as_index(cap));
> >> + }
> >> +
> >> + /**
> >> + * Determine whether or not the specified SPIR-V capability is enabled
> >> + */
> >> + inline bool is_enabled(SpvCapability cap)
> >> + {
> >> + return BITSET_TEST(capabilities, capability_as_index(cap));
> >> + }
> >> +
> >> + /**
> >> + * Reduce a set of SPIR-V capabilities to the minimal set.
> >> + *
> >> + * Many SPIR-V capabilities imply other capabilities. For example,
> >> + * \c SpvCapabilityShader implies \c SpvCapabilityMatrix, so only the
> >> + * former needs to be set. This method removes all the redundant
> >> + * capabilities so that the minimal set of \c SpvOpCapability
> >> instructions
> >> + * can be written to the output.
> >> + */
> >> + void reduce()
> >> + {
> >> + for (unsigned i = 0; i < ${len(all_values)}; ++i) {
> >> + if (BITSET_TEST(capabilities, i)) {
> >> + /* Walk back up the dependency chain clearing all the bits
> >> along
> >> + * the way. This is necessary because some of the
> >> dependencies
> >> + * might not be set, so we cannot rely on the dependency-of-a-
> >> + * dependency to be cleared automatically.
> >> + */
> >> + unsigned dep = dependency_map[i];
> >> + while (dep != NO_DEPENDENCY) {
> >> + BITSET_CLEAR(capabilities, dep);
> >> + dep = dependency_map[dep];
> >> + }
> >> + }
> >> + }
> >> + }
> >> +
> >> + spirv_capability_set_iterator begin() const
> >> + {
> >> + return spirv_capability_set_iterator(this);
> >> + }
> >> +
> >> + spirv_capability_set_iterator end() const
> >> + {
> >> + return spirv_capability_set_iterator(this, ${len(all_values)});
> >> + }
> >> +
> >> +private:
> >> + /**
> >> + * Map a SPIR-V capability to its location in the bitfield.
> >> + */
> >> + static unsigned capability_as_index(SpvCapability cap)
> >> + {
> >> + if (cap <= SpvCapability${by_value[[x for x in all_values if x <
> >> 4096][-1]][0]})
> >> + return unsigned(cap);
> >> +
> >> + switch (cap) {
> >> + % for x in all_values:
> >> + % if x >= 4096:
> >> + case SpvCapability${by_value[x][0]}: return ${all_values.index(x)};
> >> + % endif
> >> + % endfor
> >> + default:
> >> + unreachable("Invalid capability.");
> >> + }
> >> + }
> >> +
> >> + /**
> >> + * Map a location in the bitfield to the SPIR-V capability it
> >> represents.
> >> + */
> >> + static SpvCapability index_as_capability(unsigned i)
> >> + {
> >> + if (i <= ${[x for x in all_values if x < 4096][-1]})
> >> + return SpvCapability(i);
> >> +
> >> + switch (i) {
> >> + % for x in all_values:
> >> + % if x >= 4096:
> >> + case ${all_values.index(x)}: return SpvCapability${by_value[x][0]};
> >> + % endif
> >> + % endfor
> >> + default:
> >> + unreachable("Invalid capability index.");
> >> + }
> >> + }
> >> +
> >> + BITSET_DECLARE(capabilities, ${len(all_values)});
> >> +
> >> + static const ${"uint16_t" if len(all_values) > 256 else "uint8_t"}
> >> dependency_map[${len(all_values)}];
> >> +
> >> + friend class spirv_capability_set_iterator;
> >> +};
> >> +
> >> +inline spirv_capability_set_iterator::spirv_capability_set_iterator(const
> >> spirv_capability_set *_s)
> >> + : s(_s), i(0), tmp(_s->capabilities[0])
> >> +{
> >> + i = __bitset_next_set(i, &tmp, s->capabilities, ${len(all_values)});
> >> +}
> >> +
> >> +inline spirv_capability_set_iterator::spirv_capability_set_iterator(
> >> + const spirv_capability_set *_s, unsigned _i)
> >> + : s(_s), i(_i)
> >> +{
> >> + tmp = s->capabilities[unsigned(i / BITSET_WORDBITS)];
> >> + tmp &= ~((1u << (i % BITSET_WORDBITS))- 1);
> >> +}
> >> +
> >> +inline bool spirv_capability_set_iterator::operator==(const
> >> spirv_capability_set_iterator &that) const
> >> +{
> >> + return s == that.s && i == that.i;
> >> +}
> >> +
> >> +inline bool spirv_capability_set_iterator::operator!=(const
> >> spirv_capability_set_iterator &that) const
> >> +{
> >> + return !(*this == that);
> >> +}
> >> +
> >> +inline spirv_capability_set_iterator
> >> &spirv_capability_set_iterator::operator++()
> >> +{
> >> + i = __bitset_next_set(i, &tmp, s->capabilities, ${len(all_values)});
> >> + return *this;
> >> +}
> >> +
> >> +inline SpvCapability spirv_capability_set_iterator::operator*() const
> >> +{
> >> + return spirv_capability_set::index_as_capability(i);
> >> +}
> >> +
> >> +#undef NO_DEPENDENCY
> >> +
> >> +#endif /* SPIRV_CAPABILITIES_H */
> >> +""")
> >> +
> >> +
> >> +TEMPLATE_CPP = Template(COPYRIGHT + """\
> >> +#include "util/macros.h" /* for unreachable() */
> >> +#include "spirv_capabilities.h"
> >> +
> >> +#define NO_DEPENDENCY ((${"uint16_t" if len(all_values) > 256 else
> >> "uint8_t"}) ~0)
> >> +
> >> +const ${"uint16_t" if len(all_values) > 256 else "uint8_t"}
> >> spirv_capability_set::dependency_map[${len(all_values)}] = {
> >> + % for x in all_values:
> >> + % if x not in by_value or len(by_value[x][1]) == 0:
> >> + NO_DEPENDENCY${"," if all_values[-1] != x else ""}
> >> + % else:
> >> + ${all_values.index(by_name[by_value[x][1][0]][0])}${"," if
> >> all_values[-1] != x else ""}
> >> + % endif
> >> + % endfor
> >> +};
> >> +""")
> >> +
> >> +if __name__ == "__main__":
> >> + pargs = parse_args()
> >> +
> >> + spirv_info = json.JSONDecoder().decode(open(pargs.json, "r").read())
> >
> > with open(pargs.json, 'r') as f:
> > spirv_info = json.load(f)
>
> There was a reason I didn't use json.load, but I don't remember what it
> was. I'll try it this way and make sure it produces the same result.
It should. load is just a convenience wrapper.
>
> >> +
> >> + by_name, by_value = collect_data(spirv_info)
> >> +
> >> + # Assume the "core" values will be fairly tightly packed.
> >> + max_core_value = max([x for skip, (x, skip) in by_name.items() if
> >> int(x) < 4096])
> >
> > Like above _ is the idiomatic unused variable in python,
> > max_core_value = max([x for x, _ in by_name.itervalues() if int(x) < 4096])
> >
> >> + all_values = [x for x in range(max_core_value + 1)] + [cap_value for
> >> cap_value, skip in sorted(by_value.items()) if cap_value >= 4096]
> >
> > Use xrange instead of range here, and _ instead of skip
>
> I think I originally used xrange, but that doesn't exist in Python3. I
> was trying to use things that worked in both. We are eventually going
> to switch, right? :(
They are, what we'll probably do for that is either add six as a dependency
(like we do in piglit) or at the top do something like:
if sys.version[0] == '2':
range = xrange
I'm not sure yet what we'll end up doing though.
>
> >> +
> >> + if pargs.out[-1] == "p":
> >
> > This sort of automagic makes the build systems really confusing to look at.
> > I'm
> > not dead set on this but I'd really prefer to have a --gen-h and --gen-cpp
> > flag,
> > because I think that makes the meson and autotools clearer. Otherwise it
> > looks
> > like you're calling the same generator twice with the same parameters, and
> > getting different results
>
> That makes sense. I just copied this from one of Jason's generators, IIRC.
>
> >> + with open(pargs.out, 'w') as f:
> >> + f.write(TEMPLATE_CPP.render(by_name=by_name,
> >> + by_value=by_value,
> >> + all_values=all_values))
> >
> > The indentation seems a little off here.
>
> Weird... I'll check that.
>
> >> + else:
> >> + with open(pargs.out, 'w') as f:
> >> + f.write(TEMPLATE_H.render(by_name=by_name,
> >> + by_value=by_value,
> >> + all_values=all_values))
> >> --
> >> 2.9.5
> >
> > Dylan
>
signature.asc
Description: signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
