[Mesa-dev] [PATCH 1.2/8] spirv: Generate spirv_info.c
Ian Romanick
idr at freedesktop.org
Mon Jul 17 17:09:54 UTC 2017
On 07/14/2017 09:22 PM, Jason Ekstrand wrote:
> On Fri, Jul 14, 2017 at 6:39 PM, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
> From: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
>
> The old table based spirv_*_to_string functions would return NULL for
> any values "inside" the table that didn't have entries. The tables also
> needed to be updated by hand each time a new spirv.h was imported.
> Generate the file instead.
>
>
> Thanks for working on this! This is way better than having a
> hand-rolled table that requires updating.
The one unfortunate thing is that spirv.h and spirv.core.grammar.json
need to be updated together.
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
> Suggested-by: Jason Ekstrand <jason at jlekstrand.net
> <mailto:jason at jlekstrand.net>>
> ---
> I am pretty sure the scons changes are bogus... halp, plz?
>
>
> I'm going to let Emil review your autotools and write you some Android.mk.
>
>
> src/compiler/Makefile.am | 2 +
> src/compiler/Makefile.nir.am <http://Makefile.nir.am> | 1 +
> src/compiler/Makefile.sources | 4 +-
> src/compiler/Makefile.spirv.am <http://Makefile.spirv.am> | 31
> ++++++++
> src/compiler/SConscript | 1 +
> src/compiler/SConscript.spirv | 32 ++++++++
> src/compiler/spirv/spirv_info.c | 156
> -------------------------------------
> src/compiler/spirv/spirv_info_c.py | 90 +++++++++++++++++++++
> 8 files changed, 160 insertions(+), 157 deletions(-)
> create mode 100644 src/compiler/Makefile.spirv.am
> <http://Makefile.spirv.am>
> create mode 100644 src/compiler/SConscript.spirv
> delete mode 100644 src/compiler/spirv/spirv_info.c
> create mode 100644 src/compiler/spirv/spirv_info_c.py
>
> diff --git a/src/compiler/Makefile.am b/src/compiler/Makefile.am
> index 4c83365..b8f6697 100644
> --- a/src/compiler/Makefile.am
> +++ b/src/compiler/Makefile.am
> @@ -63,3 +63,5 @@ PYTHON_GEN = $(AM_V_GEN)$(PYTHON2) $(PYTHON_FLAGS)
> include Makefile.glsl.am <http://Makefile.glsl.am>
>
> include Makefile.nir.am <http://Makefile.nir.am>
> +
> +include Makefile.spirv.am <http://Makefile.spirv.am>
> diff --git a/src/compiler/Makefile.nir.am <http://Makefile.nir.am>
> b/src/compiler/Makefile.nir.am <http://Makefile.nir.am>
> index 13f02a7..d10f173 100644
> --- a/src/compiler/Makefile.nir.am <http://Makefile.nir.am>
> +++ b/src/compiler/Makefile.nir.am <http://Makefile.nir.am>
> @@ -29,6 +29,7 @@ nir_libnir_la_LIBADD = \
> nir_libnir_la_SOURCES = \
> $(NIR_FILES) \
> $(SPIRV_FILES) \
> + $(SPIRV_GENERATED_FILES) \
> $(NIR_GENERATED_FILES)
>
> nir/nir_builder_opcodes.h: nir/nir_opcodes.py
> nir/nir_builder_opcodes_h.py
> diff --git a/src/compiler/Makefile.sources
> b/src/compiler/Makefile.sources
> index d3447fb..785782b 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -277,12 +277,14 @@ NIR_FILES = \
> nir/nir_worklist.c \
> nir/nir_worklist.h
>
> +SPIRV_GENERATED_FILES = \
> + spirv/spirv_info.c
> +
> SPIRV_FILES = \
> spirv/GLSL.std.450.h \
> spirv/nir_spirv.h \
> spirv/spirv.h \
> spirv/spirv_info.h \
> - spirv/spirv_info.c \
> spirv/spirv_to_nir.c \
> spirv/vtn_alu.c \
> spirv/vtn_cfg.c \
> diff --git a/src/compiler/Makefile.spirv.am
> <http://Makefile.spirv.am> b/src/compiler/Makefile.spirv.am
> <http://Makefile.spirv.am>
> new file mode 100644
> index 0000000..6d5dfc0
> --- /dev/null
> +++ b/src/compiler/Makefile.spirv.am <http://Makefile.spirv.am>
> @@ -0,0 +1,31 @@
> +# 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 PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +spirv/spirv_info.c: spirv/spirv_info_c.py spirv/spirv.core.grammar.json
> + $(MKDIR_GEN)
> + $(PYTHON_GEN) $(srcdir)/spirv/spirv_info_c.py > $@ || ($(RM)
> $@; false)
>
>
> I'm sorry for all the pain I'm sure this caused you... Being the first
> one to add auto-generation to a directory is the worst...
It turns out that the biggest problem was me typoing SPRIV instead of
SPIRV in a couple places. Once Ken pointed that out to me, everything
worked. Sigh...
> +
> +BUILT_SOURCES += $(SPIRV_GENERATED_FILES)
> +CLEANFILES += $(SPIRV_GENERATED_FILES)
> +
> +EXTRA_DIST += \
> + spirv/spirv_info_c.py \
> + spirv/spirv.core.grammar.json
> diff --git a/src/compiler/SConscript b/src/compiler/SConscript
> index 44509a9..2797abe 100644
> --- a/src/compiler/SConscript
> +++ b/src/compiler/SConscript
> @@ -26,4 +26,5 @@ compiler = env.ConvenienceLibrary(
> Export('compiler')
>
> SConscript('SConscript.glsl')
> +SConscript('SConscript.spirv')
> SConscript('SConscript.nir')
> diff --git a/src/compiler/SConscript.spirv
> b/src/compiler/SConscript.spirv
> new file mode 100644
> index 0000000..8a90c2f
> --- /dev/null
> +++ b/src/compiler/SConscript.spirv
> @@ -0,0 +1,32 @@
> +import common
> +
> +Import('*')
> +
> +from sys import executable as python_cmd
> +
> +env = env.Clone()
> +
> +env.MSVC2013Compat()
> +
> +env.Prepend(CPPPATH = [
> + '#include',
> + '#src',
> + '#src/mapi',
> + '#src/mesa',
> + '#src/gallium/include',
> + '#src/gallium/auxiliary',
> + '#src/compiler/nir',
> +])
> +
> +# Make generated headers reachable from the include path.
> +env.Prepend(CPPPATH = [Dir('.').abspath, Dir('glsl').abspath])
> +env.Prepend(CPPPATH = [Dir('.').abspath, Dir('nir').abspath])
> +
> +# SPIR-V generated sources
> +
> +spirv_info_c = env.CodeGenerate(
> + target = 'spirv/spirv_info.c',
> + script = 'spirv/spirv_info_c.py',
> + source = [],
> + command = python_cmd + ' $SCRIPT > $TARGET'
> +)
> diff --git a/src/compiler/spirv/spirv_info.c
> b/src/compiler/spirv/spirv_info.c
> deleted file mode 100644
> index 1036b41..0000000
> --- a/src/compiler/spirv/spirv_info.c
> +++ /dev/null
> @@ -1,156 +0,0 @@
> -/*
> - * Copyright © 2016 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 PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS
> - * IN THE SOFTWARE.
> - */
> -
> -#include "spirv_info.h"
> -#include "util/macros.h"
> -
> -#define CAPABILITY(cap) [SpvCapability##cap] = #cap
> -static const char * const capability_to_string[] = {
> - CAPABILITY(Matrix),
> - CAPABILITY(Shader),
> - CAPABILITY(Geometry),
> - CAPABILITY(Tessellation),
> - CAPABILITY(Addresses),
> - CAPABILITY(Linkage),
> - CAPABILITY(Kernel),
> - CAPABILITY(Vector16),
> - CAPABILITY(Float16Buffer),
> - CAPABILITY(Float16),
> - CAPABILITY(Float64),
> - CAPABILITY(Int64),
> - CAPABILITY(Int64Atomics),
> - CAPABILITY(ImageBasic),
> - CAPABILITY(ImageReadWrite),
> - CAPABILITY(ImageMipmap),
> - CAPABILITY(Pipes),
> - CAPABILITY(Groups),
> - CAPABILITY(DeviceEnqueue),
> - CAPABILITY(LiteralSampler),
> - CAPABILITY(AtomicStorage),
> - CAPABILITY(Int16),
> - CAPABILITY(TessellationPointSize),
> - CAPABILITY(GeometryPointSize),
> - CAPABILITY(ImageGatherExtended),
> - CAPABILITY(StorageImageMultisample),
> - CAPABILITY(UniformBufferArrayDynamicIndexing),
> - CAPABILITY(SampledImageArrayDynamicIndexing),
> - CAPABILITY(StorageBufferArrayDynamicIndexing),
> - CAPABILITY(StorageImageArrayDynamicIndexing),
> - CAPABILITY(ClipDistance),
> - CAPABILITY(CullDistance),
> - CAPABILITY(ImageCubeArray),
> - CAPABILITY(SampleRateShading),
> - CAPABILITY(ImageRect),
> - CAPABILITY(SampledRect),
> - CAPABILITY(GenericPointer),
> - CAPABILITY(Int8),
> - CAPABILITY(InputAttachment),
> - CAPABILITY(SparseResidency),
> - CAPABILITY(MinLod),
> - CAPABILITY(Sampled1D),
> - CAPABILITY(Image1D),
> - CAPABILITY(SampledCubeArray),
> - CAPABILITY(SampledBuffer),
> - CAPABILITY(ImageBuffer),
> - CAPABILITY(ImageMSArray),
> - CAPABILITY(StorageImageExtendedFormats),
> - CAPABILITY(ImageQuery),
> - CAPABILITY(DerivativeControl),
> - CAPABILITY(InterpolationFunction),
> - CAPABILITY(TransformFeedback),
> - CAPABILITY(GeometryStreams),
> - CAPABILITY(StorageImageReadWithoutFormat),
> - CAPABILITY(StorageImageWriteWithoutFormat),
> - CAPABILITY(MultiViewport),
> - CAPABILITY(SubgroupDispatch),
> - CAPABILITY(NamedBarrier),
> - CAPABILITY(PipeStorage),
> - CAPABILITY(SubgroupBallotKHR),
> - CAPABILITY(DrawParameters),
> -};
> -
> -const char *
> -spirv_capability_to_string(SpvCapability cap)
> -{
> - if (cap < ARRAY_SIZE(capability_to_string))
> - return capability_to_string[cap];
> - else
> - return "unknown";
> -}
> -
> -#define DECORATION(dec) [SpvDecoration##dec] = #dec
> -static const char * const decoration_to_string[] = {
> - DECORATION(RelaxedPrecision),
> - DECORATION(SpecId),
> - DECORATION(Block),
> - DECORATION(BufferBlock),
> - DECORATION(RowMajor),
> - DECORATION(ColMajor),
> - DECORATION(ArrayStride),
> - DECORATION(MatrixStride),
> - DECORATION(GLSLShared),
> - DECORATION(GLSLPacked),
> - DECORATION(CPacked),
> - DECORATION(BuiltIn),
> - DECORATION(NoPerspective),
> - DECORATION(Flat),
> - DECORATION(Patch),
> - DECORATION(Centroid),
> - DECORATION(Sample),
> - DECORATION(Invariant),
> - DECORATION(Restrict),
> - DECORATION(Aliased),
> - DECORATION(Volatile),
> - DECORATION(Constant),
> - DECORATION(Coherent),
> - DECORATION(NonWritable),
> - DECORATION(NonReadable),
> - DECORATION(Uniform),
> - DECORATION(SaturatedConversion),
> - DECORATION(Stream),
> - DECORATION(Location),
> - DECORATION(Component),
> - DECORATION(Index),
> - DECORATION(Binding),
> - DECORATION(DescriptorSet),
> - DECORATION(Offset),
> - DECORATION(XfbBuffer),
> - DECORATION(XfbStride),
> - DECORATION(FuncParamAttr),
> - DECORATION(FPRoundingMode),
> - DECORATION(FPFastMathMode),
> - DECORATION(LinkageAttributes),
> - DECORATION(NoContraction),
> - DECORATION(InputAttachmentIndex),
> - DECORATION(Alignment),
> - DECORATION(MaxByteOffset),
> -};
> -
> -const char *
> -spirv_decoration_to_string(SpvDecoration dec)
> -{
> - if (dec < ARRAY_SIZE(decoration_to_string))
> - return decoration_to_string[dec];
> - else
> - return "unknown";
> -}
> diff --git a/src/compiler/spirv/spirv_info_c.py
> b/src/compiler/spirv/spirv_info_c.py
> new file mode 100644
> index 0000000..5af832e
> --- /dev/null
> +++ b/src/compiler/spirv/spirv_info_c.py
> @@ -0,0 +1,90 @@
> +# -*- coding: utf-8 -*-
> +
> +# Copyright © 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 PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS
> +# IN THE SOFTWARE.
>
>
> What you have is fine here but I've been really liking the style that's
> used in mesa/main/format_fallbacks.py and the anv_extensions.py that I
> sent out which does
>
> COPYRIGHT="""
> \* The usual C copyright
> */
> """
I've done this in some other places where the same copyright is used
with multiple templates. In the GLX protocol scripts, I actually had a
function that generated the copyright boilerplate parameterized with a
list of copyright holders. Maybe to reduce the copy-and-paste we should
make something like that generally available?
> and then maybe some functions and then
>
> TEMPLATE = mako.template.Template(COPYRIGHT + """
> /* My template goes here */
> """)
>
> Followed by the main function. Putting the template inside the main
> function (or if statement in this case) means the indentation gets
> really weird.
>
>
> +
> +import json
> +import mako.template
> +
> +def collect_data(spirv, kind):
> + for x in spirv["operand_kinds"]:
> + if x["kind"] == kind:
> + operands = x
>
>
> break?
Yes. I originally structured this a bit differently (finding both
Capability and Decoration in one loop), and that didn't need a break
here. I'll add it.
> +
> + # There are some duplicate values in some of the tables (thanks
> guys!), so
> + # filter them out.
>
>
> That's unfortunate...
>
>
> + last_value = -1
> + values = []
> + for x in operands["enumerants"]:
> + if x["value"] != last_value:
> + last_value = x["value"]
> + values.append(x["enumerant"])
>
>
> This assumes that the duplicate values will be sequential. Is that
> always the case? I suppose it isn't since the C compiles. Maybe better
> to use a set and do
>
> if x["value"] not in seen_values:
> seen_values.add(x["value"])
>
> What you have here works though so we can always make that change later
> if it's ever a problem.
Right now there are two pairs of duplicated values, and they appear
sequentially in the file. This happens because the names are currently
sorted by value. I don't know if that will always be the case, but I've
made that assumption in another script too. I considered a more complex
scheme for dealing with this that used another table to track names by
value, but I opted for simpler.
{
"enumerant" : "StorageBuffer16BitAccess",
"value" : 4433,
"extensions" : [ "SPV_KHR_16bit_storage" ]
},
{
"enumerant" : "StorageUniformBufferBlock16",
"value" : 4433,
"extensions" : [ "SPV_KHR_16bit_storage" ]
},
{
"enumerant" : "UniformAndStorageBuffer16BitAccess",
"value" : 4434,
"capabilities" : [
"StorageBuffer16BitAccess",
"StorageUniformBufferBlock16"
],
"extensions" : [ "SPV_KHR_16bit_storage" ]
},
{
"enumerant" : "StorageUniform16",
"value" : 4434,
"capabilities" : [
"StorageBuffer16BitAccess",
"StorageUniformBufferBlock16"
],
"extensions" : [ "SPV_KHR_16bit_storage" ]
},
One of the pairs is used as a dependency, and both are listed. This
isn't relevant for this script, but it was a hassle for another that I'm
writing.
{
"enumerant" : "UniformAndStorageBuffer16BitAccess",
"value" : 4434,
"capabilities" : [
"StorageBuffer16BitAccess",
"StorageUniformBufferBlock16"
],
"extensions" : [ "SPV_KHR_16bit_storage" ]
},
{
"enumerant" : "StorageUniform16",
"value" : 4434,
"capabilities" : [
"StorageBuffer16BitAccess",
"StorageUniformBufferBlock16"
],
"extensions" : [ "SPV_KHR_16bit_storage" ]
},
> +
> + return (kind, values)
> +
> +if __name__ == "__main__":
> + t = mako.template.Template("""/*
> + * 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 PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "spirv_info.h"
> + % for kind,values in info:
>
>
> Dylan will probably tell you to indent your C code along with the mako.
> I, personally, don't care. If anything, I would just dedent all the
> mako by a step. But I don't really care all that much. It's readable
> as-is and we don't really have a mako style guide.
>
>
> +
> +const char *
> +spirv_${kind.lower()}_to_string(Spv${kind} v)
> +{
> + switch (v) {
> + % for name in values:
> + case Spv${kind}${name}: return "Spv${kind}${name}";
> + % endfor
> + case Spv${kind}Max: break; /* silence warnings about unhandled
> enums. */
> + }
> +
> + return "unknown";
> +}
> + % endfor
> +""")
> +
> + spirv_info =
> json.JSONDecoder().decode(open("spirv/spirv.core.grammar.json",
> "r").read())
> +
> + capabilities = collect_data(spirv_info, "Capability")
> + decorations = collect_data(spirv_info, "Decoration")
> +
> + print(t.render(info=[capabilities, decorations]))
>
>
> The more recent generators have started using python's argparse and have
> --xml (or --json in this case) parameter for the input and --out for the
> output. Take a look at mesa/main/format_fallbacks.py for an example.
> Having the generator write the output file makes adding it to automake a
> bit easier to do correctly.
That's a good suggestion. I'll look into that. I wasn't terribly happy
with hard coding the path.
> --Jason
More information about the mesa-dev
mailing list