[Mesa-dev] [PATCH 1.2/8] spirv: Generate spirv_info.c

Jason Ekstrand jason at jlekstrand.net
Sat Jul 15 04:22:09 UTC 2017


On Fri, Jul 14, 2017 at 6:39 PM, Ian Romanick <idr at freedesktop.org> wrote:

> From: Ian Romanick <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.


> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Suggested-by: Jason Ekstrand <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       |   1 +
>  src/compiler/Makefile.sources      |   4 +-
>  src/compiler/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
>  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
>
>  include Makefile.nir.am
> +
> +include Makefile.spirv.am
> diff --git a/src/compiler/Makefile.nir.am b/src/compiler/Makefile.nir.am
> index 13f02a7..d10f173 100644
> --- a/src/compiler/Makefile.nir.am
> +++ b/src/compiler/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 b/src/compiler/Makefile.spirv.
> am
> new file mode 100644
> index 0000000..6d5dfc0
> --- /dev/null
> +++ b/src/compiler/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...


> +
> +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
 */
"""

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?


> +
> +    # 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.


> +
> +    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.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170714/78acbfb5/attachment-0001.html>


More information about the mesa-dev mailing list