[Mesa-dev] [PATCH 09/10] spirv: Generate code to track SPIR-V capability dependencies
From: Ian Romanick v2: Clean ups. Remove some functions that never ended up being used. v3: After updating spirv.core.grammar.json, fix the handling of ShaderViewportMaskNV. See the comment around line 71 of spirv_capabilities_h.py. Signed-off-by: Ian Romanick --- src/compiler/Makefile.sources | 2 + src/compiler/Makefile.spirv.am | 8 + src/compiler/glsl/meson.build | 3 +- src/compiler/spirv/.gitignore | 2 + src/compiler/spirv/meson.build | 14 ++ src/compiler/spirv/spirv_capabilities_h.py | 355 + 6 files changed, 383 insertions(+), 1 deletion(-) create mode 100644 src/compiler/spirv/spirv_capabilities_h.py diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources index 2ab8e16..1d67cba 100644 --- a/src/compiler/Makefile.sources +++ b/src/compiler/Makefile.sources @@ -287,6 +287,8 @@ NIR_FILES = \ nir/nir_worklist.h SPIRV_GENERATED_FILES = \ + spirv/spirv_capabilities.cpp \ + spirv/spirv_capabilities.h \ spirv/spirv_info.c SPIRV_FILES = \ diff --git a/src/compiler/Makefile.spirv.am b/src/compiler/Makefile.spirv.am index 9841004..4bc684a 100644 --- a/src/compiler/Makefile.spirv.am +++ b/src/compiler/Makefile.spirv.am @@ -20,6 +20,14 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS # IN THE SOFTWARE. +spirv/spirv_capabilities.cpp: spirv/spirv_capabilities_h.py spirv/spirv.core.grammar.json + $(MKDIR_GEN) + $(PYTHON_GEN) $(srcdir)/spirv/spirv_capabilities_h.py $(srcdir)/spirv/spirv.core.grammar.json $@ || ($(RM) $@; false) + +spirv/spirv_capabilities.h: spirv/spirv_capabilities_h.py spirv/spirv.core.grammar.json + $(MKDIR_GEN) + $(PYTHON_GEN) $(srcdir)/spirv/spirv_capabilities_h.py $(srcdir)/spirv/spirv.core.grammar.json $@ || ($(RM) $@; false) + spirv/spirv_info.c: spirv/spirv_info_c.py spirv/spirv.core.grammar.json $(MKDIR_GEN) $(PYTHON_GEN) $(srcdir)/spirv/spirv_info_c.py $(srcdir)/spirv/spirv.core.grammar.json $@ || ($(RM) $@; false) diff --git a/src/compiler/glsl/meson.build b/src/compiler/glsl/meson.build index 5b505c0..961bfb0 100644 --- a/src/compiler/glsl/meson.build +++ b/src/compiler/glsl/meson.build @@ -198,7 +198,8 @@ files_libglsl_standalone = files( libglsl = static_library( 'glsl', [files_libglsl, glsl_parser, glsl_lexer_cpp, ir_expression_operation_h, - ir_expression_operation_strings_h, ir_expression_operation_constant_h], + ir_expression_operation_strings_h, ir_expression_operation_constant_h, + spirv_capabilities_cpp, spirv_capabilities_h], c_args : [c_vis_args, c_msvc_compat_args, no_override_init_args], cpp_args : [cpp_vis_args, cpp_msvc_compat_args], link_with : [libnir, libglcpp], diff --git a/src/compiler/spirv/.gitignore b/src/compiler/spirv/.gitignore index f723c31..6b5ef0a 100644 --- a/src/compiler/spirv/.gitignore +++ b/src/compiler/spirv/.gitignore @@ -1 +1,3 @@ +/spirv_capabilities.cpp +/spirv_capabilities.h /spirv_info.c diff --git a/src/compiler/spirv/meson.build b/src/compiler/spirv/meson.build index 8f1a02e..e4ca3b6 100644 --- a/src/compiler/spirv/meson.build +++ b/src/compiler/spirv/meson.build @@ -24,3 +24,17 @@ spirv_info_c = custom_target( output : 'spirv_info.c', command : [prog_python2, '@INPUT0@', '@INPUT1@', '@OUTPUT@'], ) + +spirv_capabilities_cpp = custom_target( + 'spirv_capabilities.cpp', + input : files('spirv_capabilities_h.py', 'spirv.core.grammar.json'), + output : 'spirv_capabilities.cpp', + command : [prog_python2, '@INPUT0@', '@INPUT1@', '@OUTPUT@'], +) + +spirv_capabilities_h = custom_target( + 'spirv_capabilities.h', + input : files('spirv_capabilities_h.py', 'spirv.core.grammar.json'), + output : 'spirv_capabilities.h', + command : [prog_python2, '@INPUT0@', '@INPUT1@', '@OUTPUT@'], +) diff --git a/src/compiler/spirv/spirv_capabilities_h.py b/src/compiler/spirv/spirv_capabilities_h.py new file mode 100644 index 000..3a4a5d7 --- /dev/null +++ b/src/compiler/spirv/spirv_capabilities_h.py @@ -0,0 +1,355 @@ +COPYRIGHT = """\ +/* + * Copyright (C) 2017 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTI
Re: [Mesa-dev] [PATCH 09/10] spirv: Generate code to track SPIR-V capability dependencies
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", []) > + > +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] > + > +# 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 > +#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 operato
Re: [Mesa-dev] [PATCH 09/10] spirv: Generate code to track SPIR-V capability dependencies
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 >> +#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 bit
Re: [Mesa-dev] [PATCH 09/10] spirv: Generate code to track SPIR-V capability dependencies
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 > >> +#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_capab