[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
