On 08/03/18 19:19, Alejandro Piñeiro wrote:
ARB_gl_spirv adds the ability to use SPIR-V binaries, and a new
method, glSpecializeShader. From OpenGL 4.6 spec, section 7.2.1
"Shader Specialization", error table:
INVALID_VALUE is generated if <pEntryPoint> does not name a valid
entry point for <shader>.
INVALID_VALUE is generated if any element of <pConstantIndex>
refers to a specialization constant that does not exist in the
shader module contained in <shader>.""
But we are not really interested on creating the nir shader at that
point, and adding nir structures on the gl_program, so at that point
we are just interested on the error checking.
So we add a new method focused on just checking those errors. It still
needs to parse the binary, but skips what it is not needed, and
doesn't create the nir shader.
Can we reword the above text to simply:
ARB_gl_spirv adds the ability to use SPIR-V binaries, and a new
method, glSpecializeShader. Here we add a new function to do the
validation for this function:
From OpenGL 4.6 spec, section 7.2.1"
"Shader Specialization", error table:
INVALID_VALUE is generated if <pEntryPoint> does not name a valid
entry point for <shader>.
INVALID_VALUE is generated if any element of <pConstantIndex>
refers to a specialization constant that does not exist in the
shader module contained in <shader>.""
v2: rebase update (spirv_to_nir options added, changes on the warning
logging, and others)
v3: include passing options on common initialization, doesn't call
setjmp on common_initialization
v4: (after Jason comments):
* Rename common_initialization to vtn_builder_create
* Move validation method and their helpers to own source file.
* Create own handle_constant_decoration_cb instead of reuse existing one
v5: put vtn_build_create refactoring to their own patch (Jason)
---
src/compiler/Makefile.sources | 1 +
src/compiler/nir/meson.build | 1 +
src/compiler/spirv/gl_spirv.c | 268 ++++++++++++++++++++++++++++++++++++++
src/compiler/spirv/nir_spirv.h | 5 +
src/compiler/spirv/spirv_to_nir.c | 35 +++--
src/compiler/spirv/vtn_private.h | 6 +
6 files changed, 302 insertions(+), 14 deletions(-)
create mode 100644 src/compiler/spirv/gl_spirv.c
diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index 37340ba809e..24218985d6d 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -295,6 +295,7 @@ SPIRV_GENERATED_FILES = \
spirv/vtn_gather_types.c
SPIRV_FILES = \
+ spirv/gl_spirv.c \
spirv/GLSL.std.450.h \
spirv/nir_spirv.h \
spirv/spirv.h \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index a70c236b958..e7a94bcc1cf 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -183,6 +183,7 @@ files_libnir = files(
'nir_vla.h',
'nir_worklist.c',
'nir_worklist.h',
+ '../spirv/gl_spirv.c',
'../spirv/GLSL.std.450.h',
'../spirv/nir_spirv.h',
'../spirv/spirv.h',
diff --git a/src/compiler/spirv/gl_spirv.c b/src/compiler/spirv/gl_spirv.c
new file mode 100644
index 00000000000..e82686bfe0d
--- /dev/null
+++ b/src/compiler/spirv/gl_spirv.c
@@ -0,0 +1,268 @@
+/*
+ * 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.
+ *
+ *
+ */
+
+#include "nir_spirv.h"
+
+#include "vtn_private.h"
+#include "spirv_info.h"
+
+static bool
+vtn_validate_preamble_instruction(struct vtn_builder *b, SpvOp opcode,
+ const uint32_t *w, unsigned count)
+{
+ switch (opcode) {
+ case SpvOpSource:
+ case SpvOpSourceExtension:
+ case SpvOpSourceContinued:
+ case SpvOpExtension:
+ case SpvOpCapability:
+ case SpvOpExtInstImport:
+ case SpvOpMemoryModel:
+ case SpvOpString:
+ case SpvOpName:
+ case SpvOpMemberName:
+ case SpvOpExecutionMode:
+ case SpvOpDecorationGroup:
+ case SpvOpMemberDecorate:
+ case SpvOpGroupDecorate:
+ case SpvOpGroupMemberDecorate:
+ break;
+
+ case SpvOpEntryPoint:
+ vtn_handle_entry_point(b, w, count);
+ break;
+
+ case SpvOpDecorate:
+ vtn_handle_decoration(b, opcode, w, count);
+ break;
+
+ default:
+ return false; /* End of preamble */
+ }
+
+ return true;
+}
+
+static void
+spec_constant_decoration_cb(struct vtn_builder *b, struct vtn_value *v,
+ int member, const struct vtn_decoration *dec,
+ void *data)
+{
+ vtn_assert(member == -1);
+ if (dec->decoration != SpvDecorationSpecId)
+ return;
+
+ for (unsigned i = 0; i < b->num_specializations; i++) {
+ if (b->specializations[i].id == dec->literals[0]) {
+ b->specializations[i].defined_on_module = true;
+ return;
+ }
+ }
+}
+
+static void
+vtn_validate_handle_constant(struct vtn_builder *b, SpvOp opcode,
+ const uint32_t *w, unsigned count)
+{
+ struct vtn_value *val = vtn_push_value(b, w[2], vtn_value_type_constant);
+
+ switch (opcode) {
+ case SpvOpConstant:
+ case SpvOpConstantNull:
+ case SpvOpSpecConstantComposite:
+ case SpvOpConstantComposite:
+ /* Nothing to do here for gl_spirv needs */
+ break;
+
+ case SpvOpConstantTrue:
+ case SpvOpConstantFalse:
+ case SpvOpSpecConstantTrue:
+ case SpvOpSpecConstantFalse:
+ case SpvOpSpecConstant:
+ case SpvOpSpecConstantOp:
+ vtn_foreach_decoration(b, val, spec_constant_decoration_cb, NULL);
+ break;
+
+ case SpvOpConstantSampler:
+ vtn_fail("OpConstantSampler requires Kernel Capability");
+ break;
+
+ default:
+ vtn_fail("Unhandled opcode");
+ }
+}
+
+static bool
+vtn_validate_handle_constant_instruction(struct vtn_builder *b, SpvOp opcode,
+ const uint32_t *w, unsigned count)
+{
+ switch (opcode) {
+ case SpvOpSource:
+ case SpvOpSourceContinued:
+ case SpvOpSourceExtension:
+ case SpvOpExtension:
+ case SpvOpCapability:
+ case SpvOpExtInstImport:
+ case SpvOpMemoryModel:
+ case SpvOpEntryPoint:
+ case SpvOpExecutionMode:
+ case SpvOpString:
+ case SpvOpName:
+ case SpvOpMemberName:
+ case SpvOpDecorationGroup:
+ case SpvOpDecorate:
+ case SpvOpMemberDecorate:
+ case SpvOpGroupDecorate:
+ case SpvOpGroupMemberDecorate:
+ vtn_fail("Invalid opcode types and variables section");
+ break;
+
+ case SpvOpTypeVoid:
+ case SpvOpTypeBool:
+ case SpvOpTypeInt:
+ case SpvOpTypeFloat:
+ case SpvOpTypeVector:
+ case SpvOpTypeMatrix:
+ case SpvOpTypeImage:
+ case SpvOpTypeSampler:
+ case SpvOpTypeSampledImage:
+ case SpvOpTypeArray:
+ case SpvOpTypeRuntimeArray:
+ case SpvOpTypeStruct:
+ case SpvOpTypeOpaque:
+ case SpvOpTypePointer:
+ case SpvOpTypeFunction:
+ case SpvOpTypeEvent:
+ case SpvOpTypeDeviceEvent:
+ case SpvOpTypeReserveId:
+ case SpvOpTypeQueue:
+ case SpvOpTypePipe:
+ /* We don't need to handle types */
+ break;
+
+ case SpvOpConstantTrue:
+ case SpvOpConstantFalse:
+ case SpvOpConstant:
+ case SpvOpConstantComposite:
+ case SpvOpConstantSampler:
+ case SpvOpConstantNull:
+ case SpvOpSpecConstantTrue:
+ case SpvOpSpecConstantFalse:
+ case SpvOpSpecConstant:
+ case SpvOpSpecConstantComposite:
+ case SpvOpSpecConstantOp:
+ vtn_validate_handle_constant(b, opcode, w, count);
+ break;
+
+ case SpvOpUndef:
+ case SpvOpVariable:
+ /* We don't need to handle them */
+ break;
+
+ default:
+ return false; /* End of preamble */
+ }
+
+ return true;
+}
+
+/*
+ * Since OpenGL 4.6 you can use SPIR-V modules directly on OpenGL. One of the
+ * new methods, glSpecializeShader include some possible errors when trying to
+ * use it.
Please move the Section information to a new line as per below. It makes
this paragraph easier to read.
* From OpenGL 4.6, Section 7.2.1, "Shader Specialization":
+ *
+ * "void SpecializeShaderARB(uint shader,
+ * const char* pEntryPoint,
+ * uint numSpecializationConstants,
+ * const uint* pConstantIndex,
+ * const uint* pConstantValue);
+ * <skip>
+ *
+ * INVALID_VALUE is generated if <pEntryPoint> does not name a valid
+ * entry point for <shader>.
+ *
+ * An INVALID_VALUE error is generated if any element of pConstantIndex refers
+ * to a specialization constant that does not exist in the shader module
+ * contained in shader."
+ *
+ * We could do those checks on spirv_to_nir, but we are only interested on the
+ * full translation later, during linking. This method is a simplified version
+ * of spirv_to_nir, looking for only the checks needed by SpecializeShader.
+ *
+ * This method returns NULL if no entry point was found, and fill the
+ * nir_spirv_specialization field "defined_on_module" accordingly. Caller
+ * would need to trigger the specific errors.
+ *
+ */
+bool
+gl_spirv_validation(const uint32_t *words, size_t word_count,
+ struct nir_spirv_specialization *spec, unsigned num_spec,
+ gl_shader_stage stage, const char *entry_point_name)
+{
+ /* vtn_warn/vtn_log uses debug.func. Setting a null to prevent crash. Not
+ * need to print the warnings now, would be done later, on the real
+ * spirv_to_nir
+ */
+ const struct spirv_to_nir_options options = { .debug.func = NULL};
+ const uint32_t *word_end = words + word_count;
+
+ struct vtn_builder *b = vtn_builder_create(words, word_count,
+ stage, entry_point_name,
+ &options);
+
+ if (b == NULL)
+ return false;
+
+ /* See also _vtn_fail() */
+ if (setjmp(b->fail_jump)) {
+ ralloc_free(b);
+ return false;
+ }
+
+ words+= 5;
With the comment added for this as previously discussed and the two nits
above fixed. This patch is:
Reviewed-by: Timothy Arceri <tarc...@itsqueeze.com>
+
+ /* Search entry point from preamble */
+ words = vtn_foreach_instruction(b, words, word_end,
+ vtn_validate_preamble_instruction);
+
+ if (b->entry_point == NULL) {
+ ralloc_free(b);
+ return false;
+ }
+
+ b->specializations = spec;
+ b->num_specializations = num_spec;
+
+ /* Handle constant instructions (we don't need to handle
+ * variables or types for gl_spirv)
+ */
+ words = vtn_foreach_instruction(b, words, word_end,
+ vtn_validate_handle_constant_instruction);
+
+ ralloc_free(b);
+
+ return true;
+}
+
diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index 2b0bdaec013..87d4120c380 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -41,6 +41,7 @@ struct nir_spirv_specialization {
uint32_t data32;
uint64_t data64;
};
+ bool defined_on_module;
};
enum nir_spirv_debug_level {
@@ -70,6 +71,10 @@ struct spirv_to_nir_options {
} debug;
};
+bool gl_spirv_validation(const uint32_t *words, size_t word_count,
+ struct nir_spirv_specialization *spec, unsigned
num_spec,
+ gl_shader_stage stage, const char *entry_point_name);
+
nir_function *spirv_to_nir(const uint32_t *words, size_t word_count,
struct nir_spirv_specialization *specializations,
unsigned num_specializations,
diff --git a/src/compiler/spirv/spirv_to_nir.c
b/src/compiler/spirv/spirv_to_nir.c
index fb63df209ce..66b87c049bb 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -461,7 +461,7 @@ vtn_foreach_execution_mode(struct vtn_builder *b, struct
vtn_value *value,
}
}
-static void
+void
vtn_handle_decoration(struct vtn_builder *b, SpvOp opcode,
const uint32_t *w, unsigned count)
{
@@ -3164,6 +3164,24 @@ stage_for_execution_model(struct vtn_builder *b,
SpvExecutionModel model)
spirv_capability_to_string(cap)); \
} while(0)
+
+void
+vtn_handle_entry_point(struct vtn_builder *b, const uint32_t *w,
+ unsigned count)
+{
+ struct vtn_value *entry_point = &b->values[w[2]];
+ /* Let this be a name label regardless */
+ unsigned name_words;
+ entry_point->name = vtn_string_literal(b, &w[3], count - 3, &name_words);
+
+ if (strcmp(entry_point->name, b->entry_point_name) != 0 ||
+ stage_for_execution_model(b, w[1]) != b->entry_point_stage)
+ return;
+
+ vtn_assert(b->entry_point == NULL);
+ b->entry_point = entry_point;
+}
+
static bool
vtn_handle_preamble_instruction(struct vtn_builder *b, SpvOp opcode,
const uint32_t *w, unsigned count)
@@ -3352,20 +3370,9 @@ vtn_handle_preamble_instruction(struct vtn_builder *b,
SpvOp opcode,
w[2] == SpvMemoryModelGLSL450);
break;
- case SpvOpEntryPoint: {
- struct vtn_value *entry_point = &b->values[w[2]];
- /* Let this be a name label regardless */
- unsigned name_words;
- entry_point->name = vtn_string_literal(b, &w[3], count - 3, &name_words);
-
- if (strcmp(entry_point->name, b->entry_point_name) != 0 ||
- stage_for_execution_model(b, w[1]) != b->entry_point_stage)
- break;
-
- vtn_assert(b->entry_point == NULL);
- b->entry_point = entry_point;
+ case SpvOpEntryPoint:
+ vtn_handle_entry_point(b, w, count);
break;
- }
case SpvOpString:
vtn_push_value(b, w[1], vtn_value_type_string)->str =
diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
index a0934158df8..2219f4619dc 100644
--- a/src/compiler/spirv/vtn_private.h
+++ b/src/compiler/spirv/vtn_private.h
@@ -723,6 +723,12 @@ struct vtn_builder* vtn_builder_create(const uint32_t
*words, size_t word_count,
gl_shader_stage stage, const char
*entry_point_name,
const struct spirv_to_nir_options
*options);
+void vtn_handle_entry_point(struct vtn_builder *b, const uint32_t *w,
+ unsigned count);
+
+void vtn_handle_decoration(struct vtn_builder *b, SpvOp opcode,
+ const uint32_t *w, unsigned count);
+
static inline uint32_t
vtn_align_u32(uint32_t v, uint32_t a)
{
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
