[Mesa-dev] [PATCH 04/11] nir/spirv: add gl_spirv_validation method
Timothy Arceri
tarceri at itsqueeze.com
Fri Mar 16 11:22:08 UTC 2018
On 16/03/18 22:11, Rob Clark wrote:
> Just curious, how much different is this from what spirv-val does? We
> are going to end up using spirv-tools from clover, so if all of this
> is just to avoid a dependency on spirv-tools, is it worth it?
It seems like a pretty simple bit of code that mostly reuses existing
code. Since most drivers don't make use of clover (at least by default)
it seems worth it IMO to avoid an external dependency.
> BR,
> -R
>
> On Thu, Mar 8, 2018 at 3:19 AM, Alejandro Piñeiro <apinheiro at igalia.com> 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.
>>
>> 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. 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;
>> +
>> + /* 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)
>> {
>> --
>> 2.14.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list