Re: [Mesa-dev] [PATCH 10/10] spirv: Generate SPIR-V builder infrastructure

2017-11-13 Thread Dylan Baker
Quoting Ian Romanick (2017-11-13 16:00:08)
> 
> That's what I had first, and it did not work in some cases.  I believe
> that list==list is only True if the elements have the same order.  This
> function only requires that both lists have the same contents without
> regard for order.  I will add a comment to that effect.

Okay, that seems good then. I would have suggested a conversion to sets, but I 
just
benchmarked it and it's 30% slower than what you have.


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 10/10] spirv: Generate SPIR-V builder infrastructure

2017-11-13 Thread Ian Romanick
On 11/13/2017 02:01 PM, Dylan Baker wrote:
> Quoting Ian Romanick (2017-11-10 14:32:50)
> [snip]
>> +
>> +def can_have_extra_operands(name, enums):
>> +"""Determine whether an enum can have extra operands.
>> +
>> +Some enums, like SpvDecoration, can have extra operands with some 
>> values.
>> +Other enums, like SpvMemorySemantics, cannot.
>> +"""
>> +
>> +if name not in enums:
>> +return False
>> +
>> +for parameter in enums[name][1]:
>> +if len(parameter[2]) > 0:
> 
> Not using len() here would be faster:
> if parameter[2]:
> return True
> 
> Or, if you like a more functional approach you could do something like:
> return any(p[2] for p in enums[name][1])
> 
>> +return True
>> +
>> +return False
>> +
>> +def prototype_for_instruction_emit(inst, all_enums, bit_enums):
>> +parameters = ["spirv_program *prog"]
>> +
>> +if "operands" in inst:
>> +parameters = parameters + [declare_parameter(operand, all_enums, 
>> bit_enums) for operand in inst["operands"]]
>> +
>> +return textwrap.dedent("""\
>> +static inline void
>> +emit_Spv{name}({parameters})""".format(name=inst["opname"],
>> +   parameters=",\n
>> ".join(parameters)))
>> +
>> +
>> +def default_value(parameter, bit_enums):
>> +"""Determine whether an enum has a value that is the default when the 
>> enum is
>> +not specified.
>> +
>> +Some enums are almost always marked as optional on the instructions that
>> +use them.  Some of these, like SpvMemoryAccess, have a value that is
>> +assumed when the value is not specificed in the instruction.
>> +"""
>> +
>> +pass
> 
> This function appears to be unused.
> 
>> +
>> +def instruction_size(instruction, enums):
>> +"""Determine the size of an instruction based on its operands
>> +
>> +Instructions that have only operands without ? or * quantifiers are 
>> sized
>> +by the number of operands.  In addition, instructions that have certain
>> +BitEnum and ValueEnum parameters also have varying sizes.
>> +"""
>> +
>> +if "operands" not in instruction:
>> +# Instructions like OpNop that have no operands.  Handle these with 
>> a
>> +# trivial special case.
>> +return 1
>> +
>> +for operand in instruction["operands"]:
>> +if "quantifier" in operand:
>> +return 0
>> +elif operand["kind"] == "LiteralString":
>> +return 0
>> +elif operand["kind"] in enums and enums[operand["kind"]][0]:
>> +return 0
>> +
>> +return len(instruction["operands"]) + 1
>> +
>> +def optional_parameter_flag(operand, bit_enums):
>> +"""Return the name of the flag for an optional parameter, None otherwise
>> +
>> +Optional parameters are noted in the JSON with a "?" quantifier.  For 
>> some
>> +types, it is possible to infer that a value does not exist by examining
>> +the value.  For example, if a optional LiteralString parameter is NULL, 
>> it
>> +is not included.  The same applies for some BitEnum kinds such as
>> +ImageOperands. For cases where the existence cannot be infered from the
>> +value, a "has_foo" flag is added to the parameter list.
>> +
>> +This function returns either the has_foo name as a string or None.
>> +"""
>> +
>> +if operand["kind"] == "LiteralString":
>> +return None
>> +
>> +if operand["kind"] == "ImageOperands":
>> +return None
>> +
>> +if operand["kind"] in bit_enums:
>> +return None
>> +
>> +if "quantifier" not in operand or operand["quantifier"] != "?":
>> +return None
>> +
>> +return "has_" + operand_name(operand)
>> +
>> +def operand_name(operand):
>> +if operand["kind"] == "IdResult":
>> +return "result"
>> +elif operand["kind"] == "IdResultType":
>> +return "result_type"
>> +elif "name" in operand:
>> +if "quantifier" in operand and "+" in operand["name"]:
>> +return operand["kind"]
>> +elif operand["name"] == "'D~ref~'":
>> +return "deref"
>> +else:
>> +name=operand["name"].replace("'", "").replace(" ", 
>> "_").replace(".", "").replace("~","_").lower()
>> +return name if name not in ["default"] else "_" + name
>> +else:
>> +return operand["kind"]
>> +
>> +def list_is_same(a, b):
>> +if len(a) != len(b):
>> +return False
>> +
>> +for x in a:
>> +if x not in b:
>> +return False
>> +
>> +return True
> 
> is there a reason you canot just do `a == b`?

That's what I had first, and it did not work in some cases.  I believe
that list==list is only True if the elements have the same order.  This
function only requires that both lists have the same contents without
regard for order.  I will add a comment to that effect.

>> +
>> +def capabilities_are_same(enums):
>> +for parameter in enums[1:]:
>> 

Re: [Mesa-dev] [PATCH 10/10] spirv: Generate SPIR-V builder infrastructure

2017-11-13 Thread Dylan Baker
Quoting Ian Romanick (2017-11-10 14:32:50)
[snip]
> +
> +def can_have_extra_operands(name, enums):
> +"""Determine whether an enum can have extra operands.
> +
> +Some enums, like SpvDecoration, can have extra operands with some values.
> +Other enums, like SpvMemorySemantics, cannot.
> +"""
> +
> +if name not in enums:
> +return False
> +
> +for parameter in enums[name][1]:
> +if len(parameter[2]) > 0:

Not using len() here would be faster:
if parameter[2]:
return True

Or, if you like a more functional approach you could do something like:
return any(p[2] for p in enums[name][1])

> +return True
> +
> +return False
> +
> +def prototype_for_instruction_emit(inst, all_enums, bit_enums):
> +parameters = ["spirv_program *prog"]
> +
> +if "operands" in inst:
> +parameters = parameters + [declare_parameter(operand, all_enums, 
> bit_enums) for operand in inst["operands"]]
> +
> +return textwrap.dedent("""\
> +static inline void
> +emit_Spv{name}({parameters})""".format(name=inst["opname"],
> +   parameters=",\n
> ".join(parameters)))
> +
> +
> +def default_value(parameter, bit_enums):
> +"""Determine whether an enum has a value that is the default when the 
> enum is
> +not specified.
> +
> +Some enums are almost always marked as optional on the instructions that
> +use them.  Some of these, like SpvMemoryAccess, have a value that is
> +assumed when the value is not specificed in the instruction.
> +"""
> +
> +pass

This function appears to be unused.

> +
> +def instruction_size(instruction, enums):
> +"""Determine the size of an instruction based on its operands
> +
> +Instructions that have only operands without ? or * quantifiers are sized
> +by the number of operands.  In addition, instructions that have certain
> +BitEnum and ValueEnum parameters also have varying sizes.
> +"""
> +
> +if "operands" not in instruction:
> +# Instructions like OpNop that have no operands.  Handle these with a
> +# trivial special case.
> +return 1
> +
> +for operand in instruction["operands"]:
> +if "quantifier" in operand:
> +return 0
> +elif operand["kind"] == "LiteralString":
> +return 0
> +elif operand["kind"] in enums and enums[operand["kind"]][0]:
> +return 0
> +
> +return len(instruction["operands"]) + 1
> +
> +def optional_parameter_flag(operand, bit_enums):
> +"""Return the name of the flag for an optional parameter, None otherwise
> +
> +Optional parameters are noted in the JSON with a "?" quantifier.  For 
> some
> +types, it is possible to infer that a value does not exist by examining
> +the value.  For example, if a optional LiteralString parameter is NULL, 
> it
> +is not included.  The same applies for some BitEnum kinds such as
> +ImageOperands. For cases where the existence cannot be infered from the
> +value, a "has_foo" flag is added to the parameter list.
> +
> +This function returns either the has_foo name as a string or None.
> +"""
> +
> +if operand["kind"] == "LiteralString":
> +return None
> +
> +if operand["kind"] == "ImageOperands":
> +return None
> +
> +if operand["kind"] in bit_enums:
> +return None
> +
> +if "quantifier" not in operand or operand["quantifier"] != "?":
> +return None
> +
> +return "has_" + operand_name(operand)
> +
> +def operand_name(operand):
> +if operand["kind"] == "IdResult":
> +return "result"
> +elif operand["kind"] == "IdResultType":
> +return "result_type"
> +elif "name" in operand:
> +if "quantifier" in operand and "+" in operand["name"]:
> +return operand["kind"]
> +elif operand["name"] == "'D~ref~'":
> +return "deref"
> +else:
> +name=operand["name"].replace("'", "").replace(" ", 
> "_").replace(".", "").replace("~","_").lower()
> +return name if name not in ["default"] else "_" + name
> +else:
> +return operand["kind"]
> +
> +def list_is_same(a, b):
> +if len(a) != len(b):
> +return False
> +
> +for x in a:
> +if x not in b:
> +return False
> +
> +return True

is there a reason you canot just do `a == b`?

> +
> +def capabilities_are_same(enums):
> +for parameter in enums[1:]:
> +if not list_is_same(enums[0][1], parameter[1]):
> +return False
> +
> +return True
> +
> +def render_enable_capability(capability_list, indent="   "):
> +if len(capability_list) == 0:
> +return ""
> +
> +execution_modes = ["Kernel", "Shader", "Geometry", "Tessellation"]
> +
> +if set(capability_list) <= set(execution_modes):
> +# If all of the capabilities are execution mode capabilities, then 
> one
> +# of the 

[Mesa-dev] [PATCH 10/10] spirv: Generate SPIR-V builder infrastructure

2017-11-10 Thread Ian Romanick
From: Ian Romanick 

v2: Don't try to automatically set SpvCapabilityGeometry or
SpvCapabilityTessellation.
---
 src/compiler/Makefile.sources |   2 +
 src/compiler/Makefile.spirv.am|   4 +
 src/compiler/spirv/.gitignore |   1 +
 src/compiler/spirv/meson.build|   7 +
 src/compiler/spirv/spirv_builder.h| 205 
 src/compiler/spirv/spirv_builder_h.py | 613 ++
 6 files changed, 832 insertions(+)
 create mode 100644 src/compiler/spirv/spirv_builder.h
 create mode 100644 src/compiler/spirv/spirv_builder_h.py

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index 1d67cba..aea067f 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -287,6 +287,7 @@ NIR_FILES = \
nir/nir_worklist.h
 
 SPIRV_GENERATED_FILES = \
+   spirv/spirv_builder_functions.h \
spirv/spirv_capabilities.cpp \
spirv/spirv_capabilities.h \
spirv/spirv_info.c
@@ -295,6 +296,7 @@ SPIRV_FILES = \
spirv/GLSL.std.450.h \
spirv/nir_spirv.h \
spirv/spirv.h \
+   spirv/spirv_builder.h \
spirv/spirv_info.h \
spirv/spirv_to_nir.c \
spirv/vtn_alu.c \
diff --git a/src/compiler/Makefile.spirv.am b/src/compiler/Makefile.spirv.am
index 4bc684a..fa31313 100644
--- a/src/compiler/Makefile.spirv.am
+++ b/src/compiler/Makefile.spirv.am
@@ -20,6 +20,10 @@
 # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
 # IN THE SOFTWARE.
 
+spirv/spirv_builder_functions.h: spirv/spirv_builder_h.py 
spirv/spirv.core.grammar.json
+   $(MKDIR_GEN)
+   $(PYTHON_GEN) $(srcdir)/spirv/spirv_builder_h.py 
$(srcdir)/spirv/spirv.core.grammar.json $@ || ($(RM) $@; false)
+
 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)
diff --git a/src/compiler/spirv/.gitignore b/src/compiler/spirv/.gitignore
index 6b5ef0a..a4753c8 100644
--- a/src/compiler/spirv/.gitignore
+++ b/src/compiler/spirv/.gitignore
@@ -1,3 +1,4 @@
+/spirv_builder_functions.h
 /spirv_capabilities.cpp
 /spirv_capabilities.h
 /spirv_info.c
diff --git a/src/compiler/spirv/meson.build b/src/compiler/spirv/meson.build
index e4ca3b6..9c3f134 100644
--- a/src/compiler/spirv/meson.build
+++ b/src/compiler/spirv/meson.build
@@ -38,3 +38,10 @@ spirv_capabilities_h = custom_target(
   output : 'spirv_capabilities.h',
   command : [prog_python2, '@INPUT0@', '@INPUT1@', '@OUTPUT@'],
 )
+
+spirv_builder_functions_h = custom_target(
+  'spirv_builder_functions.h',
+  input : files('spirv_builder_h.py', 'spirv.core.grammar.json'),
+  output : 'spirv_builder_functions.h',
+  command : [prog_python2, '@INPUT0@', '@INPUT1@', '@OUTPUT@'],
+)
diff --git a/src/compiler/spirv/spirv_builder.h 
b/src/compiler/spirv/spirv_builder.h
new file mode 100644
index 000..a97cfb7
--- /dev/null
+++ b/src/compiler/spirv/spirv_builder.h
@@ -0,0 +1,205 @@
+/* -*- c++ -*- */
+/*
+ * 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.
+ */
+#ifndef SPIRV_BUILDER_H
+#define SPIRV_BUILDER_H
+
+#include 
+#include 
+#include 
+#include "spirv.h"
+#include "spirv_capabilities.h"
+
+#define INSTRUCTION_HEADER(op, size) (((size) << 16) | op)
+
+class spirv_program {
+public:
+   spirv_program(spirv_capability_set *_capabilities)
+  : capabilities(_capabilities), max_instruction(1024), instruction(0),
+begin(~0)
+   {
+  memory = (uint32_t *) calloc(max_instruction, sizeof(uint32_t));
+   }
+
+   ~spirv_program()
+   {
+  free(memory);
+   }
+
+   unsigned size_in_bytes() const
+   {
+  assert(instruction <= max_instruction);
+
+  return instruction * sizeof(uint32_t);
+   }
+
+