[Mesa-dev] [PATCH v2 00/25] Initial gl_spirv and spirv_extensions support in Mesa and i965

[Eduardo Lima Mitev]

Hello,

This is the second version of the series providing initial support for ARB_gl_spirv and ARB_spirv_extensions in Mesa and i965.

First version of the series can be found at <https://lists.freedesktop.org/archives/mesa-dev/2017-November/177004.html>.

In this series we hope we have addressed all issues detected during the initial review. Thank you all who participated!

Taking the nitpicks and minor fixes apart, most important changes compared to the first version are:

* A dedicated 'spirv' flag was removed from gl_shader struct. Now we use the nulness of 'spirv_data' member for the same purpose.

* The per-program 'spirv' flag was moved out of this series, but will likely be re-introduced in the next delivery, because it will become necessary.

* We enforce one SPIR-V shader per stage, and fail linking if this condition is not met.

* 'SpirVCapabilities' struct of GL context constants is no longer a pointer but a static struct.

As usual, a tree of this series can be found at <https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v2>.

A tree of the larger WIP branch from which this series is taken: <https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>.

Thanks in advance for the reviews!

cheers,
Eduardo

Alejandro Piñeiro (9):
spirv_extensions: rename nir_spirv_supported_extensions
mesa: move nir_spirv_supported_capabilities definition
i965: initialize SPIR-V capabilities
spirv_extensions: add GL_ARB_spirv_extensions boilerplate
spirv_extensions: add list of extensions and to_string method
spirv_extensions: define spirv_extensions_supported
spirv_extensions: add spirv_supported_extensions on gl_constants
spirv_extensions: i965: initialize SPIR-V extensions
nir/spirv: add gl_spirv_validation method

Eduardo Lima Mitev (8):
mesa/glspirv: Add struct gl_shader_spirv_data
mesa/glspirv: Add a _mesa_spirv_link_shaders() placeholder
mesa/program: Link SPIR-V shaders using the SPIR-V code-path
mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader
mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program
mesa/glspirv: Add a _mesa_spirv_to_nir() function
i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders
i965: Don't call process_glsl_ir() for SPIR-V shaders

Neil Roberts (1):
mesa: Add boilerplate for the GL 4.6 alias of glSpecializeShaderARB

Nicolai Hähnle (7):
mesa: add GL_ARB_gl_spirv boilerplate
mesa/glspirv: Add struct gl_spirv_module
mesa: implement SPIR-V loading in glShaderBinary
mesa/shaderapi: add a getter for GL_SPIR_V_BINARY_ARB
mesa: refuse to compile SPIR-V shaders or link mixed shaders
mesa: add gl_constants::SpirVCapabilities
mesa: Implement glSpecializeShaderARB

src/amd/vulkan/radv_shader.c | 4 +-
src/compiler/Makefile.sources | 2 +
src/compiler/spirv/nir_spirv.h | 21 +-
src/compiler/spirv/spirv_extensions.c | 77 +++++++
src/compiler/spirv/spirv_extensions.h | 63 ++++++
src/compiler/spirv/spirv_to_nir.c | 160 +++++++++++++-
src/compiler/spirv/vtn_private.h | 2 +-
src/intel/vulkan/anv_pipeline.c | 4 +-
src/mapi/glapi/gen/ARB_gl_spirv.xml | 21 ++
src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 ++
src/mapi/glapi/gen/GL4x.xml | 11 +
src/mapi/glapi/gen/Makefile.am | 2 +
src/mapi/glapi/gen/gl_API.xml | 8 +
src/mapi/glapi/gen/gl_genexec.py | 1 +
src/mapi/glapi/gen/meson.build | 2 +
src/mesa/Makefile.sources | 4 +
src/mesa/drivers/dri/i965/brw_context.c | 26 +++
src/mesa/drivers/dri/i965/brw_link.cpp | 3 +-
src/mesa/drivers/dri/i965/brw_program.c | 14 +-
src/mesa/main/context.c | 2 +
src/mesa/main/extensions_table.h | 2 +
src/mesa/main/get.c | 7 +
src/mesa/main/get_hash_params.py | 3 +
src/mesa/main/getstring.c | 12 +
src/mesa/main/glspirv.c | 331 ++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 108 +++++++++
src/mesa/main/mtypes.h | 31 +++
src/mesa/main/shaderapi.c | 60 ++++-
src/mesa/main/shaderobj.c | 3 +
src/mesa/main/spirv_extensions.c | 60 +++++
src/mesa/main/spirv_extensions.h | 49 ++++
src/mesa/main/tests/dispatch_sanity.cpp | 3 +
src/mesa/meson.build | 4 +
src/mesa/program/ir_to_mesa.cpp | 23 +-
34 files changed, 1098 insertions(+), 38 deletions(-)
create mode 100644 src/compiler/spirv/spirv_extensions.c
create mode 100644 src/compiler/spirv/spirv_extensions.h
create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml
create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
create mode 100644 src/mesa/main/glspirv.c
create mode 100644 src/mesa/main/glspirv.h
create mode 100644 src/mesa/main/spirv_extensions.c
create mode 100644 src/mesa/main/spirv_extensions.h

[Eduardo Lima Mitev]

From: Nicolai Hähnle <***@amd.com>

v2: * Add meson build bits (Eric Engestrom)

* Return INVALID_OPERATION error on SpecializeShaderARB (Ian Romanick)

Reviewed-by: Emil Velikov <***@collabora.com>
Reviewed-by: Ian Romanick <***@intel.com>
Reviewed-by: Timothy Arceri <***@itsqueeze.com>
---
src/mapi/glapi/gen/ARB_gl_spirv.xml | 21 ++++++++++++++
src/mapi/glapi/gen/Makefile.am | 1 +
src/mapi/glapi/gen/gl_API.xml | 4 +++
src/mapi/glapi/gen/gl_genexec.py | 1 +
src/mapi/glapi/gen/meson.build | 1 +
src/mesa/Makefile.sources | 2 ++
src/mesa/main/extensions_table.h | 1 +
src/mesa/main/glspirv.c | 39 +++++++++++++++++++++++++
src/mesa/main/glspirv.h | 51 +++++++++++++++++++++++++++++++++
src/mesa/main/mtypes.h | 1 +
src/mesa/main/tests/dispatch_sanity.cpp | 3 ++
src/mesa/meson.build | 2 ++
12 files changed, 127 insertions(+)
create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml
create mode 100644 src/mesa/main/glspirv.c
create mode 100644 src/mesa/main/glspirv.h

diff --git a/src/mapi/glapi/gen/ARB_gl_spirv.xml b/src/mapi/glapi/gen/ARB_gl_spirv.xml
new file mode 100644
index 00000000000..0dd615480f7
--- /dev/null
+++ b/src/mapi/glapi/gen/ARB_gl_spirv.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+<!DOCTYPE OpenGLAPI SYSTEM "gl_API.dtd">
+
+<OpenGLAPI>
+
+<category name="GL_ARB_gl_spirv" number="190">
+
+ <enum name="SHADER_BINARY_FORMAT_SPIR_V_ARB" value="0x9551"/>
+ <enum name="SPIR_V_BINARY_ARB" value="0x9552"/>
+
+ <function name="SpecializeShaderARB">
+ <param name="shader" type="GLuint"/>
+ <param name="pEntryPoint" type="const GLchar *"/>
+ <param name="numSpecializationConstants" type="GLuint"/>
+ <param name="pConstantIndex" type="const GLuint *"/>
+ <param name="pConstantValue" type="const GLuint *"/>
+ </function>
+
+</category>
+
+</OpenGLAPI>
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index 87d8517b7ba..35e37e95a9f 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -144,6 +144,7 @@ API_XML = \
ARB_framebuffer_object.xml \
ARB_get_program_binary.xml \
ARB_get_texture_sub_image.xml \
+ ARB_gl_spirv.xml \
ARB_gpu_shader_fp64.xml \
ARB_gpu_shader_int64.xml \
ARB_gpu_shader5.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index eb1d9b83b27..d3594cfe195 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -8400,6 +8400,10 @@

<xi:include href="ARB_gpu_shader_int64.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>

+<!-- ARB extension 179 - 189 -->
+
+<xi:include href="ARB_gl_spirv.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
+
<!-- Non-ARB extensions sorted by extension number. -->

<category name="GL_EXT_blend_color" number="2">
diff --git a/src/mapi/glapi/gen/gl_genexec.py b/src/mapi/glapi/gen/gl_genexec.py
index b7b22328ff8..aaff9f230b3 100644
--- a/src/mapi/glapi/gen/gl_genexec.py
+++ b/src/mapi/glapi/gen/gl_genexec.py
@@ -77,6 +77,7 @@ header = """/**
#include "main/eval.h"
#include "main/externalobjects.h"
#include "main/get.h"
+#include "main/glspirv.h"
#include "main/feedback.h"
#include "main/fog.h"
#include "main/fbobject.h"
diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build
index 599f094e998..a6a93cc83be 100644
--- a/src/mapi/glapi/gen/meson.build
+++ b/src/mapi/glapi/gen/meson.build
@@ -52,6 +52,7 @@ api_xml_files = files(
'ARB_framebuffer_object.xml',
'ARB_get_program_binary.xml',
'ARB_get_texture_sub_image.xml',
+ 'ARB_gl_spirv.xml',
'ARB_gpu_shader_fp64.xml',
'ARB_gpu_shader_int64.xml',
'ARB_gpu_shader5.xml',
diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
index 6da1e3fef9d..e9680bf004c 100644
--- a/src/mesa/Makefile.sources
+++ b/src/mesa/Makefile.sources
@@ -118,6 +118,8 @@ MAIN_FILES = \
main/getstring.c \
main/glformats.c \
main/glformats.h \
+ main/glspirv.c \
+ main/glspirv.h \
main/glthread.c \
main/glthread.h \
main/glheader.h \
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index 5b66e7d30df..ab15ceb9414 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -72,6 +72,7 @@ EXT(ARB_framebuffer_object , ARB_framebuffer_object
EXT(ARB_framebuffer_sRGB , EXT_framebuffer_sRGB , GLL, GLC, x , x , 1998)
EXT(ARB_get_program_binary , dummy_true , GLL, GLC, x , x , 2010)
EXT(ARB_get_texture_sub_image , dummy_true , GLL, GLC, x , x , 2014)
+EXT(ARB_gl_spirv , ARB_gl_spirv , x, GLC, x , x , 2016)
EXT(ARB_gpu_shader5 , ARB_gpu_shader5 , x , GLC, x , x , 2010)
EXT(ARB_gpu_shader_fp64 , ARB_gpu_shader_fp64 , x , GLC, x , x , 2010)
EXT(ARB_gpu_shader_int64 , ARB_gpu_shader_int64 , x , GLC, x , x , 2015)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
new file mode 100644
index 00000000000..3989f424241
--- /dev/null
+++ b/src/mesa/main/glspirv.c
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2017 Advanced Micro Devices, Inc.
+ *
+ * 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
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, 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 NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHOR(S) AND/OR THEIR SUPPLIERS 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 "glspirv.h"
+
+#include "errors.h"
+
+void GLAPIENTRY
+_mesa_SpecializeShaderARB(GLuint shader,
+ const GLchar *pEntryPoint,
+ GLuint numSpecializationConstants,
+ const GLuint *pConstantIndex,
+ const GLuint *pConstantValue)
+{
+ GET_CURRENT_CONTEXT(ctx);
+
+ /* Just return GL_INVALID_OPERATION error while this is boilerplate */
+ _mesa_error(ctx, GL_INVALID_OPERATION, "SpecializeShaderARB");
+}
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
new file mode 100644
index 00000000000..1de88717faa
--- /dev/null
+++ b/src/mesa/main/glspirv.h
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2017 Advanced Micro Devices, Inc.
+ *
+ * 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
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, 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 NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHOR(S) AND/OR THEIR SUPPLIERS 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 GLSPIRV_H
+#define GLSPIRV_H
+
+#include "mtypes.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * \name API functions
+ */
+/*@{*/
+
+void GLAPIENTRY
+_mesa_SpecializeShaderARB(GLuint shader,
+ const GLchar *pEntryPoint,
+ GLuint numSpecializationConstants,
+ const GLuint *pConstantIndex,
+ const GLuint *pConstantValue);
+
+/*@}*/
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* GLSPIRV_H */
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 0e8a05359a4..062eea609c7 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4062,6 +4062,7 @@ struct gl_extensions
GLboolean ARB_enhanced_layouts;
GLboolean ARB_explicit_attrib_location;
GLboolean ARB_explicit_uniform_location;
+ GLboolean ARB_gl_spirv;
GLboolean ARB_gpu_shader5;
GLboolean ARB_gpu_shader_fp64;
GLboolean ARB_gpu_shader_int64;
diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp
index ae46419ec48..b2ff35717b7 100644
--- a/src/mesa/main/tests/dispatch_sanity.cpp
+++ b/src/mesa/main/tests/dispatch_sanity.cpp
@@ -1963,6 +1963,9 @@ const struct function gl_core_functions_possible[] = {
{ "glProgramUniform3ui64vARB", 45, -1 },
{ "glProgramUniform4ui64vARB", 45, -1 },

+ /* GL_ARB_gl_spirv */
+ { "glSpecializeShaderARB", 45, -1 },
+
{ NULL, 0, -1 }
};

diff --git a/src/mesa/meson.build b/src/mesa/meson.build
index 05a3a9ac55d..2eec7d45f7d 100644
--- a/src/mesa/meson.build
+++ b/src/mesa/meson.build
@@ -162,6 +162,8 @@ files_libmesa_common = files(
'main/getstring.c',
'main/glformats.c',
'main/glformats.h',
+ 'main/glspirv.c',
+ 'main/glspirv.h',
'main/glthread.c',
'main/glthread.h',
'main/glheader.h',

[Eduardo Lima Mitev]

From: Neil Roberts <***@igalia.com>

---
src/mapi/glapi/gen/GL4x.xml | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/src/mapi/glapi/gen/GL4x.xml b/src/mapi/glapi/gen/GL4x.xml
index 88dba5cd71a..0a8094166c8 100644
--- a/src/mapi/glapi/gen/GL4x.xml
+++ b/src/mapi/glapi/gen/GL4x.xml
@@ -73,6 +73,17 @@
<param name="clamp" type="GLfloat"/>
</function>
<enum name="POLYGON_OFFSET_CLAMP" value="0x8E1B"/>
+
+ <enum name="SHADER_BINARY_FORMAT_SPIR_V" value="0x9551"/>
+ <enum name="SPIR_V_BINARY" value="0x9552"/>
+
+ <function name="SpecializeShader" alias="SpecializeShaderARB">
+ <param name="shader" type="GLuint"/>
+ <param name="pEntryPoint" type="const GLchar *"/>
+ <param name="numSpecializationConstants" type="GLuint"/>
+ <param name="pConstantIndex" type="const GLuint *"/>
+ <param name="pConstantValue" type="const GLuint *"/>
+ </function>
</category>

</OpenGLAPI>

[Ian Romanick]

I'd squash this in with the previous patch.

Post by Eduardo Lima Mitev
---
src/mapi/glapi/gen/GL4x.xml | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/src/mapi/glapi/gen/GL4x.xml b/src/mapi/glapi/gen/GL4x.xml
index 88dba5cd71a..0a8094166c8 100644
--- a/src/mapi/glapi/gen/GL4x.xml
+++ b/src/mapi/glapi/gen/GL4x.xml
@@ -73,6 +73,17 @@
<param name="clamp" type="GLfloat"/>
</function>
<enum name="POLYGON_OFFSET_CLAMP" value="0x8E1B"/>
+
+ <enum name="SHADER_BINARY_FORMAT_SPIR_V" value="0x9551"/>
+ <enum name="SPIR_V_BINARY" value="0x9552"/>
+
+ <function name="SpecializeShader" alias="SpecializeShaderARB">
+ <param name="shader" type="GLuint"/>
+ <param name="pEntryPoint" type="const GLchar *"/>
+ <param name="numSpecializationConstants" type="GLuint"/>
+ <param name="pConstantIndex" type="const GLuint *"/>
+ <param name="pConstantValue" type="const GLuint *"/>
+ </function>
</category>
</OpenGLAPI>

[Eduardo Lima Mitev]

From: Nicolai Hähnle <***@amd.com>

v2: * Make the SPIR-V module struct part of a larger gl_shader_spirv_data
struct that will be introduced later, and don't reference it directly
in gl_shader. (Eduardo Lima)
* Readability improvements (Ian Romanick)

Reviewed-by: Ian Romanick <***@intel.com>
---
src/mesa/main/glspirv.c | 17 +++++++++++++++++
src/mesa/main/glspirv.h | 16 ++++++++++++++++
2 files changed, 33 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 3989f424241..d4832db549d 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -25,6 +25,23 @@

#include "errors.h"

+#include "util/u_atomic.h"
+
+void
+_mesa_spirv_module_reference(struct gl_spirv_module **dest,
+ struct gl_spirv_module *src)
+{
+ struct gl_spirv_module *old = *dest;
+
+ if (old && p_atomic_dec_zero(&old->RefCount))
+ free(old);
+
+ *dest = src;
+
+ if (src)
+ p_atomic_inc(&src->RefCount);
+}
+
void GLAPIENTRY
_mesa_SpecializeShaderARB(GLuint shader,
const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index 1de88717faa..4e033735cfe 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -30,6 +30,22 @@
extern "C" {
#endif

+/**
+ * A SPIR-V module contains the raw SPIR-V binary as set by ShaderBinary.
+ *
+ * It is reference-counted, because the same module can be attached to multiple
+ * shader objects simultaneously.
+ */
+struct gl_spirv_module {
+ unsigned RefCount;
+ GLint Length;
+ char Binary[0];
+};
+
+void
+_mesa_spirv_module_reference(struct gl_spirv_module **dest,
+ struct gl_spirv_module *src);
+
/**
* \name API functions
*/

[Eduardo Lima Mitev]

This is a per-shader structure holding the SPIR-V data associated with the
shader (binary module, specialization constants and entry-point).

This is needed because both gl_shader and gl_linked_shader need to share this
data. Instead of copying the data, we pass a reference to it upon program
linking. That's why it is reference-counted.

This struct is created and associated with the shader upon calling
glShaderBinary(), then subsequently filled up by the call to
glSpecializeShaderARB().

v2: Readability improvements (Ian Romanick)

Reviewed-by: Ian Romanick <***@intel.com>
---
src/mesa/main/glspirv.c | 17 +++++++++++++++++
src/mesa/main/glspirv.h | 25 +++++++++++++++++++++++++
2 files changed, 42 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index d4832db549d..8d1e652e088 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -42,6 +42,23 @@ _mesa_spirv_module_reference(struct gl_spirv_module **dest,
p_atomic_inc(&src->RefCount);
}

+void
+_mesa_shader_spirv_data_reference(struct gl_shader_spirv_data **dest,
+ struct gl_shader_spirv_data *src)
+{
+ struct gl_shader_spirv_data *old = *dest;
+
+ if (old && p_atomic_dec_zero(&old->RefCount)) {
+ _mesa_spirv_module_reference(&(*dest)->SpirVModule, NULL);
+ ralloc_free(old);
+ }
+
+ *dest = src;
+
+ if (src)
+ p_atomic_inc(&src->RefCount);
+}
+
void GLAPIENTRY
_mesa_SpecializeShaderARB(GLuint shader,
const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index 4e033735cfe..b8a0125ea9f 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -42,10 +42,35 @@ struct gl_spirv_module {
char Binary[0];
};

+/**
+ * SPIR-V data needed to compile and link a SPIR-V shader.
+ *
+ * It includes a SPIR-V binary that is potentially shared among different
+ * shaders; and shader-specific specialization constants and entry point.
+ *
+ * It is reference-counted because it is shared between gl_shader and its
+ * corresponding gl_linked_shader.
+ */
+struct gl_shader_spirv_data {
+ GLint RefCount;
+
+ struct gl_spirv_module *SpirVModule;
+
+ GLchar *SpirVEntryPoint;
+
+ GLuint NumSpecializationConstants;
+ GLuint *SpecializationConstantsIndex;
+ GLuint *SpecializationConstantsValue;
+};
+
void
_mesa_spirv_module_reference(struct gl_spirv_module **dest,
struct gl_spirv_module *src);

+void
+_mesa_shader_spirv_data_reference(struct gl_shader_spirv_data **dest,
+ struct gl_shader_spirv_data *src);
+
/**
* \name API functions
*/

[Eduardo Lima Mitev]

From: Nicolai Hähnle <***@amd.com>

v2: * Add a gl_shader_spirv_data member to gl_shader, which already
encapsulates a gl_spirv_module where the binary will be saved.
(Eduardo Lima)

* Just use the 'spirv_data' member to know whether a gl_shader has
the SPIR_V_BINARY_ARB state. (Timothy Arceri)

* Remove redundant argument checks. Move extension presence check
to API entry point where the rest of checks are. Retype 'n' and
'length'arguments to use the correct and more standard types.
(Ian Romanick)
---
src/mesa/main/glspirv.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 5 +++++
src/mesa/main/mtypes.h | 4 ++++
src/mesa/main/shaderapi.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
src/mesa/main/shaderobj.c | 2 ++
5 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 8d1e652e088..d2e76bb1927 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -25,6 +25,8 @@

#include "errors.h"

+#include "errors.h"
+
#include "util/u_atomic.h"

void
@@ -59,6 +61,47 @@ _mesa_shader_spirv_data_reference(struct gl_shader_spirv_data **dest,
p_atomic_inc(&src->RefCount);
}

+void
+_mesa_spirv_shader_binary(struct gl_context *ctx,
+ unsigned n, struct gl_shader **shaders,
+ const void* binary, size_t length)
+{
+ struct gl_spirv_module *module;
+ struct gl_shader_spirv_data *spirv_data;
+
+ assert(length >= 0);
+
+ module = malloc(sizeof(*module) + (size_t)length);
+ if (!module) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderBinary");
+ return;
+ }
+
+ p_atomic_set(&module->RefCount, 0);
+ module->Length = length;
+ memcpy(&module->Binary[0], binary, length);
+
+ for (int i = 0; i < n; ++i) {
+ struct gl_shader *sh = shaders[i];
+
+ spirv_data = rzalloc(NULL, struct gl_shader_spirv_data);
+ _mesa_shader_spirv_data_reference(&sh->spirv_data, spirv_data);
+ _mesa_spirv_module_reference(&spirv_data->SpirVModule, module);
+
+ sh->CompileStatus = compile_failure;
+
+ free((void *)sh->Source);
+ sh->Source = NULL;
+ free((void *)sh->FallbackSource);
+ sh->FallbackSource = NULL;
+
+ ralloc_free(sh->ir);
+ sh->ir = NULL;
+ ralloc_free(sh->symbols);
+ sh->symbols = NULL;
+ }
+}
+
void GLAPIENTRY
_mesa_SpecializeShaderARB(GLuint shader,
const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index b8a0125ea9f..ba281f68bef 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -71,6 +71,11 @@ void
_mesa_shader_spirv_data_reference(struct gl_shader_spirv_data **dest,
struct gl_shader_spirv_data *src);

+void
+_mesa_spirv_shader_binary(struct gl_context *ctx,
+ unsigned n, struct gl_shader **shaders,
+ const void* binary, size_t length);
+
/**
* \name API functions
*/
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 062eea609c7..50a47e0a65d 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -98,6 +98,7 @@ struct st_context;
struct gl_uniform_storage;
struct prog_instruction;
struct gl_program_parameter_list;
+struct gl_shader_spirv_data;
struct set;
struct set_entry;
struct vbo_context;
@@ -2646,6 +2647,9 @@ struct gl_shader
GLuint TransformFeedbackBufferStride[MAX_FEEDBACK_BUFFERS];

struct gl_shader_info info;
+
+ /* ARB_gl_spirv related data */
+ struct gl_shader_spirv_data *spirv_data;
};


diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 72824355838..24058e5ee2e 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -42,6 +42,7 @@
#include "main/context.h"
#include "main/dispatch.h"
#include "main/enums.h"
+#include "main/glspirv.h"
#include "main/hash.h"
#include "main/mtypes.h"
#include "main/pipelineobj.h"
@@ -1051,6 +1052,16 @@ set_shader_source(struct gl_shader *sh, const GLchar *source)
{
assert(sh);

+ /* The GL_ARB_gl_spirv spec adds the following to the end of the description
+ * of ShaderSource:
+ *
+ * "If <shader> was previously associated with a SPIR-V module (via the
+ * ShaderBinary command), that association is broken. Upon successful
+ * completion of this command the SPIR_V_BINARY_ARB state of <shader>
+ * is set to FALSE."
+ */
+ _mesa_shader_spirv_data_reference(&sh->spirv_data, NULL);
+
if (sh->CompileStatus == compile_skipped && !sh->FallbackSource) {
/* If shader was previously compiled back-up the source in case of cache
* fallback.
@@ -2132,9 +2143,7 @@ _mesa_ShaderBinary(GLint n, const GLuint* shaders, GLenum binaryformat,
const void* binary, GLint length)
{
GET_CURRENT_CONTEXT(ctx);
- (void) shaders;
- (void) binaryformat;
- (void) binary;
+ struct gl_shader **sh;

/* Page 68, section 7.2 'Shader Binaries" of the of the OpenGL ES 3.1, and
* page 88 of the OpenGL 4.5 specs state:
@@ -2148,6 +2157,36 @@ _mesa_ShaderBinary(GLint n, const GLuint* shaders, GLenum binaryformat,
return;
}

+ /* Get all shader objects at once so we can make the operation
+ * all-or-nothing.
+ */
+ if (n > SIZE_MAX / sizeof(*sh)) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderBinary(count)");
+ return;
+ }
+
+ sh = alloca(sizeof(*sh) * (size_t)n);
+ if (!sh) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderBinary");
+ return;
+ }
+
+ for (int i = 0; i < n; ++i) {
+ sh[i] = _mesa_lookup_shader_err(ctx, shaders[i], "glShaderBinary");
+ if (!sh[i])
+ return;
+ }
+
+ if (binaryformat == GL_SHADER_BINARY_FORMAT_SPIR_V_ARB) {
+ if (!ctx->Extensions.ARB_gl_spirv)
+ _mesa_error(ctx, GL_INVALID_OPERATION, "glShaderBinary(SPIR-V)");
+ else if (n > 0)
+ _mesa_spirv_shader_binary(ctx, (unsigned) n, sh, binary,
+ (size_t) length);
+
+ return;
+ }
+
_mesa_error(ctx, GL_INVALID_ENUM, "glShaderBinary(format)");
}

diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index ce2e3df4fae..5c1cdd6b27a 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -33,6 +33,7 @@
#include "compiler/glsl/string_to_uint_map.h"
#include "main/glheader.h"
#include "main/context.h"
+#include "main/glspirv.h"
#include "main/hash.h"
#include "main/mtypes.h"
#include "main/shaderapi.h"
@@ -121,6 +122,7 @@ _mesa_new_shader(GLuint name, gl_shader_stage stage)
void
_mesa_delete_shader(struct gl_context *ctx, struct gl_shader *sh)
{
+ _mesa_shader_spirv_data_reference(&sh->spirv_data, NULL);
free((void *)sh->Source);
free((void *)sh->FallbackSource);
free(sh->Label);

[Ian Romanick]

Two nits below...

Post by Eduardo Lima Mitev
v2: * Add a gl_shader_spirv_data member to gl_shader, which already
encapsulates a gl_spirv_module where the binary will be saved.
(Eduardo Lima)
* Just use the 'spirv_data' member to know whether a gl_shader has
the SPIR_V_BINARY_ARB state. (Timothy Arceri)
* Remove redundant argument checks. Move extension presence check
to API entry point where the rest of checks are. Retype 'n' and
'length'arguments to use the correct and more standard types.
(Ian Romanick)
---
src/mesa/main/glspirv.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 5 +++++
src/mesa/main/mtypes.h | 4 ++++
src/mesa/main/shaderapi.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
src/mesa/main/shaderobj.c | 2 ++
5 files changed, 96 insertions(+), 3 deletions(-)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 8d1e652e088..d2e76bb1927 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -25,6 +25,8 @@
#include "errors.h"
+#include "errors.h"
+
#include "util/u_atomic.h"
void
@@ -59,6 +61,47 @@ _mesa_shader_spirv_data_reference(struct gl_shader_spirv_data **dest,
p_atomic_inc(&src->RefCount);
}
+void
+_mesa_spirv_shader_binary(struct gl_context *ctx,
+ unsigned n, struct gl_shader **shaders,
+ const void* binary, size_t length)
+{
+ struct gl_spirv_module *module;
+ struct gl_shader_spirv_data *spirv_data;
+
+ assert(length >= 0);
+
+ module = malloc(sizeof(*module) + (size_t)length);

Don't need the (size_t) because you made length be size_t. :)

Post by Eduardo Lima Mitev
+ if (!module) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderBinary");
+ return;
+ }
+
+ p_atomic_set(&module->RefCount, 0);
+ module->Length = length;
+ memcpy(&module->Binary[0], binary, length);
+
+ for (int i = 0; i < n; ++i) {
+ struct gl_shader *sh = shaders[i];
+
+ spirv_data = rzalloc(NULL, struct gl_shader_spirv_data);
+ _mesa_shader_spirv_data_reference(&sh->spirv_data, spirv_data);
+ _mesa_spirv_module_reference(&spirv_data->SpirVModule, module);
+
+ sh->CompileStatus = compile_failure;
+
+ free((void *)sh->Source);
+ sh->Source = NULL;
+ free((void *)sh->FallbackSource);
+ sh->FallbackSource = NULL;
+
+ ralloc_free(sh->ir);
+ sh->ir = NULL;
+ ralloc_free(sh->symbols);
+ sh->symbols = NULL;
+ }
+}
+
void GLAPIENTRY
_mesa_SpecializeShaderARB(GLuint shader,
const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index b8a0125ea9f..ba281f68bef 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -71,6 +71,11 @@ void
_mesa_shader_spirv_data_reference(struct gl_shader_spirv_data **dest,
struct gl_shader_spirv_data *src);
+void
+_mesa_spirv_shader_binary(struct gl_context *ctx,
+ unsigned n, struct gl_shader **shaders,
+ const void* binary, size_t length);
+
/**
* \name API functions
*/
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 062eea609c7..50a47e0a65d 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -98,6 +98,7 @@ struct st_context;
struct gl_uniform_storage;
struct prog_instruction;
struct gl_program_parameter_list;
+struct gl_shader_spirv_data;
struct set;
struct set_entry;
struct vbo_context;
@@ -2646,6 +2647,9 @@ struct gl_shader
GLuint TransformFeedbackBufferStride[MAX_FEEDBACK_BUFFERS];
struct gl_shader_info info;
+
+ /* ARB_gl_spirv related data */
+ struct gl_shader_spirv_data *spirv_data;
};
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 72824355838..24058e5ee2e 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -42,6 +42,7 @@
#include "main/context.h"
#include "main/dispatch.h"
#include "main/enums.h"
+#include "main/glspirv.h"
#include "main/hash.h"
#include "main/mtypes.h"
#include "main/pipelineobj.h"
@@ -1051,6 +1052,16 @@ set_shader_source(struct gl_shader *sh, const GLchar *source)
{
assert(sh);
+ /* The GL_ARB_gl_spirv spec adds the following to the end of the description
+ *
+ * "If <shader> was previously associated with a SPIR-V module (via the
+ * ShaderBinary command), that association is broken. Upon successful
+ * completion of this command the SPIR_V_BINARY_ARB state of <shader>
+ * is set to FALSE."
+ */
+ _mesa_shader_spirv_data_reference(&sh->spirv_data, NULL);
+
if (sh->CompileStatus == compile_skipped && !sh->FallbackSource) {
/* If shader was previously compiled back-up the source in case of cache
* fallback.
@@ -2132,9 +2143,7 @@ _mesa_ShaderBinary(GLint n, const GLuint* shaders, GLenum binaryformat,
const void* binary, GLint length)
{
GET_CURRENT_CONTEXT(ctx);
- (void) shaders;
- (void) binaryformat;
- (void) binary;
+ struct gl_shader **sh;
/* Page 68, section 7.2 'Shader Binaries" of the of the OpenGL ES 3.1, and
@@ -2148,6 +2157,36 @@ _mesa_ShaderBinary(GLint n, const GLuint* shaders, GLenum binaryformat,
return;
}
+ /* Get all shader objects at once so we can make the operation
+ * all-or-nothing.
+ */
+ if (n > SIZE_MAX / sizeof(*sh)) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderBinary(count)");
+ return;
+ }
+
+ sh = alloca(sizeof(*sh) * (size_t)n);
+ if (!sh) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderBinary");
+ return;
+ }
+
+ for (int i = 0; i < n; ++i) {
+ sh[i] = _mesa_lookup_shader_err(ctx, shaders[i], "glShaderBinary");
+ if (!sh[i])
+ return;
+ }
+
+ if (binaryformat == GL_SHADER_BINARY_FORMAT_SPIR_V_ARB) {
+ if (!ctx->Extensions.ARB_gl_spirv)
+ _mesa_error(ctx, GL_INVALID_OPERATION, "glShaderBinary(SPIR-V)");
+ else if (n > 0)
+ _mesa_spirv_shader_binary(ctx, (unsigned) n, sh, binary,
+ (size_t) length);

This block should get enclosed in { } because it's more than one line.

With those fixed, this patch is

Post by Eduardo Lima Mitev
+
+ return;
+ }
+
_mesa_error(ctx, GL_INVALID_ENUM, "glShaderBinary(format)");
}
diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index ce2e3df4fae..5c1cdd6b27a 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -33,6 +33,7 @@
#include "compiler/glsl/string_to_uint_map.h"
#include "main/glheader.h"
#include "main/context.h"
+#include "main/glspirv.h"
#include "main/hash.h"
#include "main/mtypes.h"
#include "main/shaderapi.h"
@@ -121,6 +122,7 @@ _mesa_new_shader(GLuint name, gl_shader_stage stage)
void
_mesa_delete_shader(struct gl_context *ctx, struct gl_shader *sh)
{
+ _mesa_shader_spirv_data_reference(&sh->spirv_data, NULL);
free((void *)sh->Source);
free((void *)sh->FallbackSource);
free(sh->Label);

[Jon Turney]

Post by Eduardo Lima Mitev
v2: * Add a gl_shader_spirv_data member to gl_shader, which already
encapsulates a gl_spirv_module where the binary will be saved.
(Eduardo Lima)
* Just use the 'spirv_data' member to know whether a gl_shader has
the SPIR_V_BINARY_ARB state. (Timothy Arceri)
* Remove redundant argument checks. Move extension presence check
to API entry point where the rest of checks are. Retype 'n' and
'length'arguments to use the correct and more standard types.
(Ian Romanick)
---
src/mesa/main/glspirv.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 5 +++++
src/mesa/main/mtypes.h | 4 ++++
src/mesa/main/shaderapi.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
src/mesa/main/shaderobj.c | 2 ++
5 files changed, 96 insertions(+), 3 deletions(-)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 8d1e652e088..d2e76bb1927 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c

[...]

Post by Eduardo Lima Mitev
+
+ sh = alloca(sizeof(*sh) * (size_t)n);
+ if (!sh) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glShaderBinary");
+ return;
+ }
+

This adds a use of alloca() without a corresponding #include.

Patch attached.

[Eduardo Lima Mitev]

From: Nicolai Hähnle <***@amd.com>

v2: Use the 'spirv_data' member of gl_shader instead of a
dedicated flag. (Timothy Arceri)
---
src/mesa/main/shaderapi.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 24058e5ee2e..3ac1419b7ee 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -961,6 +961,9 @@ get_shaderiv(struct gl_context *ctx, GLuint name, GLenum pname, GLint *params)
case GL_SHADER_SOURCE_LENGTH:
*params = shader->Source ? strlen((char *) shader->Source) + 1 : 0;
break;
+ case GL_SPIR_V_BINARY_ARB:
+ *params = (shader->spirv_data != NULL);
+ break;
default:
_mesa_error(ctx, GL_INVALID_ENUM, "glGetShaderiv(pname)");
return;

[Ian Romanick]

This patch is

Post by Eduardo Lima Mitev
v2: Use the 'spirv_data' member of gl_shader instead of a
dedicated flag. (Timothy Arceri)
---
src/mesa/main/shaderapi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 24058e5ee2e..3ac1419b7ee 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -961,6 +961,9 @@ get_shaderiv(struct gl_context *ctx, GLuint name, GLenum pname, GLint *params)
*params = shader->Source ? strlen((char *) shader->Source) + 1 : 0;
break;
+ *params = (shader->spirv_data != NULL);
+ break;
_mesa_error(ctx, GL_INVALID_ENUM, "glGetShaderiv(pname)");
return;

[Eduardo Lima Mitev]

From: Nicolai Hähnle <***@amd.com>

Note that gl_shader::CompileStatus will also indicate whether a shader
has been successfully specialized.

v2: Use the 'spirv_data' member of gl_shader to know if it is a SPIR-V
shader, instead of a dedicated flag. (Timothy Arceri)
---
src/mesa/main/shaderapi.c | 12 ++++++++++++
src/mesa/program/ir_to_mesa.cpp | 17 ++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 3ac1419b7ee..251c876ada8 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1092,6 +1092,18 @@ _mesa_compile_shader(struct gl_context *ctx, struct gl_shader *sh)
if (!sh)
return;

+ /* The GL_ARB_gl_spirv spec says:
+ *
+ * "Add a new error for the CompileShader command:
+ *
+ * An INVALID_OPERATION error is generated if the SPIR_V_BINARY_ARB
+ * state of <shader> is TRUE."
+ */
+ if (sh->spirv_data) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "glCompileShader(SPIR-V)");
+ return;
+ }
+
if (!sh->Source) {
/* If the user called glCompileShader without first calling
* glShaderSource, we should fail to compile, but not raise a GL_ERROR.
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index aa8b6d7084b..047f5b38f71 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -3077,6 +3077,7 @@ void
_mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
{
unsigned int i;
+ GLboolean spirv;

_mesa_clear_shader_program_data(ctx, prog);

@@ -3086,7 +3087,21 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)

for (i = 0; i < prog->NumShaders; i++) {
if (!prog->Shaders[i]->CompileStatus) {
- linker_error(prog, "linking with uncompiled shader");
+ linker_error(prog, "linking with uncompiled/unspecialized shader");
+ }
+
+ if (!i) {
+ spirv = (prog->Shaders[i]->spirv_data != NULL);
+ } else if (spirv && !prog->Shaders[i]->spirv_data) {
+ /* The GL_ARB_gl_spirv spec adds a new bullet point to the list of
+ * reasons LinkProgram can fail:
+ *
+ * "All the shader objects attached to <program> do not have the
+ * same value for the SPIR_V_BINARY_ARB state."
+ */
+ linker_error(prog,
+ "not all attached shaders have the same "
+ "SPIR_V_BINARY_ARB state");
}
}

[Ian Romanick]

Post by Eduardo Lima Mitev
Note that gl_shader::CompileStatus will also indicate whether a shader
has been successfully specialized.
v2: Use the 'spirv_data' member of gl_shader to know if it is a SPIR-V
shader, instead of a dedicated flag. (Timothy Arceri)
---
src/mesa/main/shaderapi.c | 12 ++++++++++++
src/mesa/program/ir_to_mesa.cpp | 17 ++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 3ac1419b7ee..251c876ada8 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1092,6 +1092,18 @@ _mesa_compile_shader(struct gl_context *ctx, struct gl_shader *sh)
if (!sh)
return;
+ *
+ *
+ * An INVALID_OPERATION error is generated if the SPIR_V_BINARY_ARB
+ * state of <shader> is TRUE."
+ */
+ if (sh->spirv_data) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "glCompileShader(SPIR-V)");
+ return;
+ }
+
if (!sh->Source) {
/* If the user called glCompileShader without first calling
* glShaderSource, we should fail to compile, but not raise a GL_ERROR.
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index aa8b6d7084b..047f5b38f71 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -3077,6 +3077,7 @@ void
_mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
{
unsigned int i;
+ GLboolean spirv;

Use bool. With that fixed, this patch is

Post by Eduardo Lima Mitev
_mesa_clear_shader_program_data(ctx, prog);
@@ -3086,7 +3087,21 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
for (i = 0; i < prog->NumShaders; i++) {
if (!prog->Shaders[i]->CompileStatus) {
- linker_error(prog, "linking with uncompiled shader");
+ linker_error(prog, "linking with uncompiled/unspecialized shader");
+ }
+
+ if (!i) {
+ spirv = (prog->Shaders[i]->spirv_data != NULL);
+ } else if (spirv && !prog->Shaders[i]->spirv_data) {
+ /* The GL_ARB_gl_spirv spec adds a new bullet point to the list of
+ *
+ * "All the shader objects attached to <program> do not have the
+ * same value for the SPIR_V_BINARY_ARB state."
+ */
+ linker_error(prog,
+ "not all attached shaders have the same "
+ "SPIR_V_BINARY_ARB state");
}
}

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

Renamed to nir_spirv_supported_capabilities.

The original name seemed to suggest that it was directly related to
the SPIR-V extensions supported, but that is not the case. For
example, float64 was supported on SPIR-V 1.0 core, without the need of
any extra extension.

Additionally, this is used at spirv_to_nir to check if a given
capability is supported or not (see spv_check_supported), not if a
given extension is supported or not.

One could argue that it should be renamed to something like
nir_spirv_supported_extra_capabilities (or similar) as not all the
capabilities are flagged there. In any case, that name seemed too long.

This rename was triggered by the need of really maintain the SPIR-V
supported extensions as part of ARB_spirv_extensions implementation,
making that struct name confusing.

Reviewed-by: Ian Romanick <***@intel.com>
---
src/amd/vulkan/radv_shader.c | 4 ++--
src/compiler/spirv/nir_spirv.h | 4 ++--
src/compiler/spirv/spirv_to_nir.c | 6 +++---
src/compiler/spirv/vtn_private.h | 2 +-
src/intel/vulkan/anv_pipeline.c | 4 ++--
5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 32edf2abd22..cea61333ebc 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -196,7 +196,7 @@ radv_shader_compile_to_nir(struct radv_device *device,
spec_entries[i].data32 = *(const uint32_t *)data;
}
}
- const struct nir_spirv_supported_extensions supported_ext = {
+ const struct nir_spirv_supported_capabilities supported_cap = {
.draw_parameters = true,
.float64 = true,
.image_read_without_format = true,
@@ -208,7 +208,7 @@ radv_shader_compile_to_nir(struct radv_device *device,
};
entry_point = spirv_to_nir(spirv, module->size / 4,
spec_entries, num_spec_entries,
- stage, entrypoint_name, &supported_ext, &nir_options);
+ stage, entrypoint_name, &supported_cap, &nir_options);
nir = entry_point->shader;
assert(nir->info.stage == stage);
nir_validate_shader(nir);
diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index 83577fb5d23..0204e81d091 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -42,7 +42,7 @@ struct nir_spirv_specialization {
};
};

-struct nir_spirv_supported_extensions {
+struct nir_spirv_supported_capabilities {
bool float64;
bool image_ms_array;
bool tessellation;
@@ -58,7 +58,7 @@ nir_function *spirv_to_nir(const uint32_t *words, size_t word_count,
struct nir_spirv_specialization *specializations,
unsigned num_specializations,
gl_shader_stage stage, const char *entry_point_name,
- const struct nir_spirv_supported_extensions *ext,
+ const struct nir_spirv_supported_capabilities *cap,
const nir_shader_compiler_options *options);

#ifdef __cplusplus
diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c
index 027efab88d7..6034228ed36 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -2672,7 +2672,7 @@ stage_for_execution_model(SpvExecutionModel model)
}

#define spv_check_supported(name, cap) do { \
- if (!(b->ext && b->ext->name)) \
+ if (!(b->cap && b->cap->name)) \
vtn_warn("Unsupported SPIR-V capability: %s", \
spirv_capability_to_string(cap)); \
} while(0)
@@ -3313,7 +3313,7 @@ nir_function *
spirv_to_nir(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,
- const struct nir_spirv_supported_extensions *ext,
+ const struct nir_spirv_supported_capabilities *cap,
const nir_shader_compiler_options *options)
{
const uint32_t *word_end = words + word_count;
@@ -3336,7 +3336,7 @@ spirv_to_nir(const uint32_t *words, size_t word_count,
exec_list_make_empty(&b->functions);
b->entry_point_stage = stage;
b->entry_point_name = entry_point_name;
- b->ext = ext;
+ b->cap = cap;

/* Handle all the preamble instructions */
words = vtn_foreach_instruction(b, words, word_end,
diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
index 6b4645acc8b..0c1ce21dd88 100644
--- a/src/compiler/spirv/vtn_private.h
+++ b/src/compiler/spirv/vtn_private.h
@@ -465,7 +465,7 @@ struct vtn_builder {

nir_shader *shader;
nir_function_impl *impl;
- const struct nir_spirv_supported_extensions *ext;
+ const struct nir_spirv_supported_capabilities *cap;
struct vtn_block *block;

/* Current file, line, and column. Useful for debugging. Set
diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
index 907b24a758d..35d68f2d658 100644
--- a/src/intel/vulkan/anv_pipeline.c
+++ b/src/intel/vulkan/anv_pipeline.c
@@ -132,7 +132,7 @@ anv_shader_compile_to_nir(struct anv_pipeline *pipeline,
}
}

- const struct nir_spirv_supported_extensions supported_ext = {
+ const struct nir_spirv_supported_capabilities supported_cap = {
.float64 = device->instance->physicalDevice.info.gen >= 8,
.int64 = device->instance->physicalDevice.info.gen >= 8,
.tessellation = true,
@@ -145,7 +145,7 @@ anv_shader_compile_to_nir(struct anv_pipeline *pipeline,
nir_function *entry_point =
spirv_to_nir(spirv, module->size / 4,
spec_entries, num_spec_entries,
- stage, entrypoint_name, &supported_ext, nir_options);
+ stage, entrypoint_name, &supported_cap, nir_options);
nir_shader *nir = entry_point->shader;
assert(nir->info.stage == stage);
nir_validate_shader(nir);

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

Due gl_spirv we will use it on more places, specifically on
gl_constants, where we would like to use it without a pointer.
---
src/compiler/spirv/nir_spirv.h | 15 ++-------------
src/mesa/main/mtypes.h | 11 +++++++++++
2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index 0204e81d091..a14b55cdd4b 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -28,7 +28,8 @@
#ifndef _NIR_SPIRV_H_
#define _NIR_SPIRV_H_

-#include "nir/nir.h"
+#include "compiler/nir/nir.h"
+#include "main/mtypes.h"

#ifdef __cplusplus
extern "C" {
@@ -42,18 +43,6 @@ struct nir_spirv_specialization {
};
};

-struct nir_spirv_supported_capabilities {
- bool float64;
- bool image_ms_array;
- bool tessellation;
- bool draw_parameters;
- bool image_read_without_format;
- bool image_write_without_format;
- bool int64;
- bool multiview;
- bool variable_pointers;
-};
-
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/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 50a47e0a65d..c8177c9a99a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3583,6 +3583,17 @@ struct gl_program_constants
GLuint MaxShaderStorageBlocks;
};

+struct nir_spirv_supported_capabilities {
+ bool float64;
+ bool image_ms_array;
+ bool tessellation;
+ bool draw_parameters;
+ bool image_read_without_format;
+ bool image_write_without_format;
+ bool int64;
+ bool multiview;
+ bool variable_pointers;
+};

/**
* Constants which may be overridden by device driver during context creation

[Timothy Arceri]

Can we get away with forward declaring this?

There is a section at the top of mtypes you can add it to:

* \name Some forward type declarations

Post by Eduardo Lima Mitev
Due gl_spirv we will use it on more places, specifically on
gl_constants, where we would like to use it without a pointer.
---
src/compiler/spirv/nir_spirv.h | 15 ++-------------
src/mesa/main/mtypes.h | 11 +++++++++++
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index 0204e81d091..a14b55cdd4b 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -28,7 +28,8 @@
#ifndef _NIR_SPIRV_H_
#define _NIR_SPIRV_H_
-#include "nir/nir.h"
+#include "compiler/nir/nir.h"
+#include "main/mtypes.h"
#ifdef __cplusplus
extern "C" {
@@ -42,18 +43,6 @@ struct nir_spirv_specialization {
};
};
-struct nir_spirv_supported_capabilities {
- bool float64;
- bool image_ms_array;
- bool tessellation;
- bool draw_parameters;
- bool image_read_without_format;
- bool image_write_without_format;
- bool int64;
- bool multiview;
- bool variable_pointers;
-};
-
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/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 50a47e0a65d..c8177c9a99a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3583,6 +3583,17 @@ struct gl_program_constants
GLuint MaxShaderStorageBlocks;
};
+struct nir_spirv_supported_capabilities {
+ bool float64;
+ bool image_ms_array;
+ bool tessellation;
+ bool draw_parameters;
+ bool image_read_without_format;
+ bool image_write_without_format;
+ bool int64;
+ bool multiview;
+ bool variable_pointers;
+};
/**
* Constants which may be overridden by device driver during context creation

[Alejandro Piñeiro]

Post by Timothy Arceri
Can we get away with forward declaring this?
 * \name Some forward type declarations

Yes, I realized that, and tried, but I still got several build errors.
So that would not be enough.

In any case, after all the recent changes on spirv/spirv_to_nir
codebase, this commit and the following one are obsolete. We are
preparing a v3 series, but meanwhile we send this path alone to mesa-dev:
https://lists.freedesktop.org/archives/mesa-dev/2017-December/179438.html

Post by Timothy Arceri

Post by Eduardo Lima Mitev
Due gl_spirv we will use it on more places, specifically on
gl_constants, where we would like to use it without a pointer.
---
  src/compiler/spirv/nir_spirv.h | 15 ++-------------
  src/mesa/main/mtypes.h         | 11 +++++++++++
  2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/compiler/spirv/nir_spirv.h
b/src/compiler/spirv/nir_spirv.h
index 0204e81d091..a14b55cdd4b 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -28,7 +28,8 @@
  #ifndef _NIR_SPIRV_H_
  #define _NIR_SPIRV_H_
  -#include "nir/nir.h"
+#include "compiler/nir/nir.h"
+#include "main/mtypes.h"
    #ifdef __cplusplus
  extern "C" {
@@ -42,18 +43,6 @@ struct nir_spirv_specialization {
     };
  };
  -struct nir_spirv_supported_capabilities {
-   bool float64;
-   bool image_ms_array;
-   bool tessellation;
-   bool draw_parameters;
-   bool image_read_without_format;
-   bool image_write_without_format;
-   bool int64;
-   bool multiview;
-   bool variable_pointers;
-};
-
  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/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 50a47e0a65d..c8177c9a99a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3583,6 +3583,17 @@ struct gl_program_constants
     GLuint MaxShaderStorageBlocks;
  };
  +struct nir_spirv_supported_capabilities {
+   bool float64;
+   bool image_ms_array;
+   bool tessellation;
+   bool draw_parameters;
+   bool image_read_without_format;
+   bool image_write_without_format;
+   bool int64;
+   bool multiview;
+   bool variable_pointers;
+};
    /**
   * Constants which may be overridden by device driver during
context creation

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Alejandro Piñeiro]

Post by Alejandro Piñeiro

Post by Timothy Arceri
Can we get away with forward declaring this?
 * \name Some forward type declarations

Yes, I realized that, and tried, but I still got several build errors.
So that would not be enough.
In any case, after all the recent changes on spirv/spirv_to_nir
codebase, this commit and the following one are obsolete.

sorry, and error: this commit and the *previous* one

Post by Alejandro Piñeiro
We are
https://lists.freedesktop.org/archives/mesa-dev/2017-December/179438.html

Post by Timothy Arceri

Post by Eduardo Lima Mitev
Due gl_spirv we will use it on more places, specifically on
gl_constants, where we would like to use it without a pointer.
---
  src/compiler/spirv/nir_spirv.h | 15 ++-------------
  src/mesa/main/mtypes.h         | 11 +++++++++++
  2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/compiler/spirv/nir_spirv.h
b/src/compiler/spirv/nir_spirv.h
index 0204e81d091..a14b55cdd4b 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -28,7 +28,8 @@
  #ifndef _NIR_SPIRV_H_
  #define _NIR_SPIRV_H_
  -#include "nir/nir.h"
+#include "compiler/nir/nir.h"
+#include "main/mtypes.h"
    #ifdef __cplusplus
  extern "C" {
@@ -42,18 +43,6 @@ struct nir_spirv_specialization {
     };
  };
  -struct nir_spirv_supported_capabilities {
-   bool float64;
-   bool image_ms_array;
-   bool tessellation;
-   bool draw_parameters;
-   bool image_read_without_format;
-   bool image_write_without_format;
-   bool int64;
-   bool multiview;
-   bool variable_pointers;
-};
-
  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/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 50a47e0a65d..c8177c9a99a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3583,6 +3583,17 @@ struct gl_program_constants
     GLuint MaxShaderStorageBlocks;
  };
  +struct nir_spirv_supported_capabilities {
+   bool float64;
+   bool image_ms_array;
+   bool tessellation;
+   bool draw_parameters;
+   bool image_read_without_format;
+   bool image_write_without_format;
+   bool int64;
+   bool multiview;
+   bool variable_pointers;
+};
    /**
   * Constants which may be overridden by device driver during
context creation

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Timothy Arceri]

Post by Alejandro Piñeiro

Post by Timothy Arceri
Can we get away with forward declaring this?
 * \name Some forward type declarations

Yes, I realized that, and tried, but I still got several build errors.
So that would not be enough.

Doesn't that just mean you need to include compiler/spirv/nir_spirv.h in
more places?

Post by Alejandro Piñeiro
In any case, after all the recent changes on spirv/spirv_to_nir
codebase, this commit and the following one are obsolete. We are
https://lists.freedesktop.org/archives/mesa-dev/2017-December/179438.html

I'm confused. If it's obsolete why are you trying to get it committed?

Post by Alejandro Piñeiro

Post by Timothy Arceri

Post by Eduardo Lima Mitev
Due gl_spirv we will use it on more places, specifically on
gl_constants, where we would like to use it without a pointer.
---
  src/compiler/spirv/nir_spirv.h | 15 ++-------------
  src/mesa/main/mtypes.h         | 11 +++++++++++
  2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/compiler/spirv/nir_spirv.h
b/src/compiler/spirv/nir_spirv.h
index 0204e81d091..a14b55cdd4b 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -28,7 +28,8 @@
  #ifndef _NIR_SPIRV_H_
  #define _NIR_SPIRV_H_
  -#include "nir/nir.h"
+#include "compiler/nir/nir.h"
+#include "main/mtypes.h"
    #ifdef __cplusplus
  extern "C" {
@@ -42,18 +43,6 @@ struct nir_spirv_specialization {
     };
  };
  -struct nir_spirv_supported_capabilities {
-   bool float64;
-   bool image_ms_array;
-   bool tessellation;
-   bool draw_parameters;
-   bool image_read_without_format;
-   bool image_write_without_format;
-   bool int64;
-   bool multiview;
-   bool variable_pointers;
-};
-
  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/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 50a47e0a65d..c8177c9a99a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3583,6 +3583,17 @@ struct gl_program_constants
     GLuint MaxShaderStorageBlocks;
  };
  +struct nir_spirv_supported_capabilities {
+   bool float64;
+   bool image_ms_array;
+   bool tessellation;
+   bool draw_parameters;
+   bool image_read_without_format;
+   bool image_write_without_format;
+   bool int64;
+   bool multiview;
+   bool variable_pointers;
+};
    /**
   * Constants which may be overridden by device driver during
context creation

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Alejandro Piñeiro]

Post by Timothy Arceri

Post by Alejandro Piñeiro

Post by Timothy Arceri
Can we get away with forward declaring this?
  * \name Some forward type declarations

Yes, I realized that, and tried, but I still got several build errors.
So that would not be enough.

Doesn't that just mean you need to include compiler/spirv/nir_spirv.h
in more places?

Post by Alejandro Piñeiro
In any case, after all the recent changes on spirv/spirv_to_nir
codebase, this commit and the following one are obsolete. We are
https://lists.freedesktop.org/archives/mesa-dev/2017-December/179438.html

I'm confused. If it's obsolete why are you trying to get it committed?

Sorry for the confusion. We sent this v2 series last week. They became
obsolete this week (on Monday). This is the reason I sent a new patch
today. As I was sending the patch, I should have send a warning for
those two patches.

Post by Timothy Arceri

Post by Alejandro Piñeiro

Post by Timothy Arceri

Post by Eduardo Lima Mitev
Due gl_spirv we will use it on more places, specifically on
gl_constants, where we would like to use it without a pointer.
---
   src/compiler/spirv/nir_spirv.h | 15 ++-------------
   src/mesa/main/mtypes.h         | 11 +++++++++++
   2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/compiler/spirv/nir_spirv.h
b/src/compiler/spirv/nir_spirv.h
index 0204e81d091..a14b55cdd4b 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -28,7 +28,8 @@
   #ifndef _NIR_SPIRV_H_
   #define _NIR_SPIRV_H_
   -#include "nir/nir.h"
+#include "compiler/nir/nir.h"
+#include "main/mtypes.h"
     #ifdef __cplusplus
   extern "C" {
@@ -42,18 +43,6 @@ struct nir_spirv_specialization {
      };
   };
   -struct nir_spirv_supported_capabilities {
-   bool float64;
-   bool image_ms_array;
-   bool tessellation;
-   bool draw_parameters;
-   bool image_read_without_format;
-   bool image_write_without_format;
-   bool int64;
-   bool multiview;
-   bool variable_pointers;
-};
-
   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/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 50a47e0a65d..c8177c9a99a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3583,6 +3583,17 @@ struct gl_program_constants
      GLuint MaxShaderStorageBlocks;
   };
   +struct nir_spirv_supported_capabilities {
+   bool float64;
+   bool image_ms_array;
+   bool tessellation;
+   bool draw_parameters;
+   bool image_read_without_format;
+   bool image_write_without_format;
+   bool int64;
+   bool multiview;
+   bool variable_pointers;
+};
     /**
    * Constants which may be overridden by device driver during
context creation

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Alejandro Piñeiro]

Post by Timothy Arceri

Post by Alejandro Piñeiro

Post by Timothy Arceri
Can we get away with forward declaring this?
  * \name Some forward type declarations

Yes, I realized that, and tried, but I still got several build errors.
So that would not be enough.

Doesn't that just mean you need to include compiler/spirv/nir_spirv.h
in more places?

Sorry, didn't realize this question.

No. The problem is that with the next patch, we are adding a variable
with that type, not a pointer. If we were just adding a pointer to
nir_spirv_supported_capabilities, then the forward definition would be
enough. If not we are getting "incomplete type" errors. Also adding an
include to compiler/spirv/nir_spirv.h on mtypes.h creates tons of
problems, as that one includes nir.h and so on.

Post by Timothy Arceri

Post by Alejandro Piñeiro
In any case, after all the recent changes on spirv/spirv_to_nir
codebase, this commit and the following one are obsolete. We are
https://lists.freedesktop.org/archives/mesa-dev/2017-December/179438.html

I'm confused. If it's obsolete why are you trying to get it committed?

Post by Alejandro Piñeiro

Post by Timothy Arceri

Post by Eduardo Lima Mitev
Due gl_spirv we will use it on more places, specifically on
gl_constants, where we would like to use it without a pointer.
---
   src/compiler/spirv/nir_spirv.h | 15 ++-------------
   src/mesa/main/mtypes.h         | 11 +++++++++++
   2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/compiler/spirv/nir_spirv.h
b/src/compiler/spirv/nir_spirv.h
index 0204e81d091..a14b55cdd4b 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -28,7 +28,8 @@
   #ifndef _NIR_SPIRV_H_
   #define _NIR_SPIRV_H_
   -#include "nir/nir.h"
+#include "compiler/nir/nir.h"
+#include "main/mtypes.h"
     #ifdef __cplusplus
   extern "C" {
@@ -42,18 +43,6 @@ struct nir_spirv_specialization {
      };
   };
   -struct nir_spirv_supported_capabilities {
-   bool float64;
-   bool image_ms_array;
-   bool tessellation;
-   bool draw_parameters;
-   bool image_read_without_format;
-   bool image_write_without_format;
-   bool int64;
-   bool multiview;
-   bool variable_pointers;
-};
-
   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/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 50a47e0a65d..c8177c9a99a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3583,6 +3583,17 @@ struct gl_program_constants
      GLuint MaxShaderStorageBlocks;
   };
   +struct nir_spirv_supported_capabilities {
+   bool float64;
+   bool image_ms_array;
+   bool tessellation;
+   bool draw_parameters;
+   bool image_read_without_format;
+   bool image_write_without_format;
+   bool int64;
+   bool multiview;
+   bool variable_pointers;
+};
     /**
    * Constants which may be overridden by device driver during
context creation

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Eduardo Lima Mitev]

From: Nicolai Hähnle <***@amd.com>

For drivers to declare which SPIR-V features they support.

v2: Don't use a pointer (Ian Romanick)
---
src/mesa/main/mtypes.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index c8177c9a99a..7fed85a2ae6 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4028,6 +4028,9 @@ struct gl_constants

/** When drivers are OK with mapped buffers during draw and other calls. */
bool AllowMappedBuffersDuringExecution;
+
+ /** GL_ARB_gl_spirv */
+ struct nir_spirv_supported_capabilities SpirVCapabilities;
};

[Timothy Arceri]

If you can forward declare nir_spirv_supported_capabilities as per my
comment on the last patch, and you add the forward declaration to this

Post by Eduardo Lima Mitev
For drivers to declare which SPIR-V features they support.
v2: Don't use a pointer (Ian Romanick)
---
src/mesa/main/mtypes.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index c8177c9a99a..7fed85a2ae6 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4028,6 +4028,9 @@ struct gl_constants
/** When drivers are OK with mapped buffers during draw and other calls. */
bool AllowMappedBuffersDuringExecution;
+
+ /** GL_ARB_gl_spirv */
+ struct nir_spirv_supported_capabilities SpirVCapabilities;
};

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

Needed for ARB_gl_spirv. Right now those are the same that the intel
vulkan driver, but those are not shared. From the ARB_spirv_extensions
spec:

"3. If a new GL extension is added that includes SPIR-V support via
a new SPIR-V extension does it's SPIR-V extension also get
enumerated by the SPIR_V_EXTENSIONS_ARB query?.

RESOLVED. Yes. It's good to include it for consistency. Any SPIR-V
functionality supported beyond the SPIR-V version that is required
for the GL API version should be enumerated."

Reading between lines, there is the possibility of specific GL
extensions enabling specific SPIR-V extensions (so capabilities). That
would mean that it is possible that OpenGL and Vulkan not having the
same capabilities supported, even for the same driver. So for now we
keep them separate. Perhaps in the future it is better to keep them
the same and synced.

v2:
* Rebase update (SpirVCapabilities not a pointer anymore)
* Fill spirv capabilities for OpenGL >= 3.3 (Ian Romanick)
---
src/mesa/drivers/dri/i965/brw_context.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index b62852d90c8..d90b7797a7a 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -75,6 +75,7 @@
#include "util/debug.h"
#include "isl/isl.h"

+#include "compiler/spirv/nir_spirv.h"
/***************************************
* Mesa's Driver Functions
***************************************/
@@ -331,6 +332,21 @@ brw_init_driver_functions(struct brw_context *brw,
functions->GetSamplePosition = gen6_get_sample_position;
}

+static void
+brw_initialize_spirv_supported_capabilities(struct brw_context *brw)
+{
+ const struct gen_device_info *devinfo = &brw->screen->devinfo;
+ struct gl_context *ctx = &brw->ctx;
+
+ ctx->Const.SpirVCapabilities.float64 = devinfo->gen >= 8;
+ ctx->Const.SpirVCapabilities.int64 = devinfo->gen >= 8;
+ ctx->Const.SpirVCapabilities.tessellation = true;
+ ctx->Const.SpirVCapabilities.draw_parameters = true;
+ ctx->Const.SpirVCapabilities.image_write_without_format = true;
+ ctx->Const.SpirVCapabilities.multiview = true;
+ ctx->Const.SpirVCapabilities.variable_pointers = true;
+}
+
static void
brw_initialize_context_constants(struct brw_context *brw)
{
@@ -696,6 +712,10 @@ brw_initialize_context_constants(struct brw_context *brw)

if (!(ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT))
ctx->Const.AllowMappedBuffersDuringExecution = true;
+
+ /* GL_ARB_gl_spirv */
+ if (ctx->Version >= 33)
+ brw_initialize_spirv_supported_capabilities(brw);
}

static void

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

v2:
* Mention extension gap at gl_API.xml (Emil Velikov)
* Bail with INVALID_ENUM if extension not available on getStringi (Emil Velikov)
* Use EXTRA_EXT macro when defining the extension at
get.c/get_hash_params.py (Emil Velikov)
* Rename source files (spirvextensions.[ch] -> spirv_extensions.[ch]) (Ian)

Reviewed-by: Ian Romanick <***@intel.com>
---
src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 ++++++++
src/mapi/glapi/gen/Makefile.am | 1 +
src/mapi/glapi/gen/gl_API.xml | 4 +++
src/mapi/glapi/gen/meson.build | 1 +
src/mesa/Makefile.sources | 2 ++
src/mesa/main/extensions_table.h | 1 +
src/mesa/main/get.c | 7 +++++
src/mesa/main/get_hash_params.py | 3 ++
src/mesa/main/getstring.c | 12 +++++++
src/mesa/main/mtypes.h | 1 +
src/mesa/main/spirv_extensions.c | 42 +++++++++++++++++++++++++
src/mesa/main/spirv_extensions.h | 49 +++++++++++++++++++++++++++++
src/mesa/meson.build | 2 ++
13 files changed, 138 insertions(+)
create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
create mode 100644 src/mesa/main/spirv_extensions.c
create mode 100644 src/mesa/main/spirv_extensions.h

diff --git a/src/mapi/glapi/gen/ARB_spirv_extensions.xml b/src/mapi/glapi/gen/ARB_spirv_extensions.xml
new file mode 100644
index 00000000000..103393104c2
--- /dev/null
+++ b/src/mapi/glapi/gen/ARB_spirv_extensions.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+<!DOCTYPE OpenGLAPI SYSTEM "gl_API.dtd">
+
+<OpenGLAPI>
+
+<category name="GL_ARB_spirv_extensions" number="194">
+
+ <enum name="SPIR_V_EXTENSIONS" value="0x9553"/>
+ <enum name="NUM_SPIR_V_EXTENSIONS" value="0x9554"/>
+
+</category>
+
+</OpenGLAPI>
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index 35e37e95a9f..9a7a268adbf 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -167,6 +167,7 @@ API_XML = \
ARB_shader_subroutine.xml \
ARB_shader_storage_buffer_object.xml \
ARB_sparse_buffer.xml \
+ ARB_spirv_extensions.xml \
ARB_sync.xml \
ARB_tessellation_shader.xml \
ARB_texture_barrier.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index d3594cfe195..00cf83eca03 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -8404,6 +8404,10 @@

<xi:include href="ARB_gl_spirv.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>

+<!-- ARB extensions 191 - 193 -->
+
+<xi:include href="ARB_spirv_extensions.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
+
<!-- Non-ARB extensions sorted by extension number. -->

<category name="GL_EXT_blend_color" number="2">
diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build
index a6a93cc83be..bfc766f7944 100644
--- a/src/mapi/glapi/gen/meson.build
+++ b/src/mapi/glapi/gen/meson.build
@@ -75,6 +75,7 @@ api_xml_files = files(
'ARB_shader_subroutine.xml',
'ARB_shader_storage_buffer_object.xml',
'ARB_sparse_buffer.xml',
+ 'ARB_spirv_extensions.xml',
'ARB_sync.xml',
'ARB_tessellation_shader.xml',
'ARB_texture_barrier.xml',
diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
index e9680bf004c..a897d72f226 100644
--- a/src/mesa/Makefile.sources
+++ b/src/mesa/Makefile.sources
@@ -203,6 +203,8 @@ MAIN_FILES = \
main/shader_query.cpp \
main/shared.c \
main/shared.h \
+ main/spirv_extensions.c \
+ main/spirv_extensions.h \
main/state.c \
main/state.h \
main/stencil.c \
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index ab15ceb9414..06deabd0640 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -129,6 +129,7 @@ EXT(ARB_shading_language_420pack , ARB_shading_language_420pack
EXT(ARB_shading_language_packing , ARB_shading_language_packing , GLL, GLC, x , x , 2011)
EXT(ARB_shadow , ARB_shadow , GLL, x , x , x , 2001)
EXT(ARB_sparse_buffer , ARB_sparse_buffer , GLL, GLC, x , x , 2014)
+EXT(ARB_spirv_extensions , ARB_spirv_extensions , x, GLC, x , x , 2016)
EXT(ARB_stencil_texturing , ARB_stencil_texturing , GLL, GLC, x , x , 2012)
EXT(ARB_sync , ARB_sync , GLL, GLC, x , x , 2003)
EXT(ARB_tessellation_shader , ARB_tessellation_shader , x , GLC, x , x , 2009)
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index ea8d932b182..2440f47558d 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -34,6 +34,7 @@
#include "get.h"
#include "macros.h"
#include "mtypes.h"
+#include "spirv_extensions.h"
#include "state.h"
#include "texcompress.h"
#include "texstate.h"
@@ -501,6 +502,7 @@ EXTRA_EXT(OES_primitive_bounding_box);
EXTRA_EXT(ARB_compute_variable_group_size);
EXTRA_EXT(KHR_robustness);
EXTRA_EXT(ARB_sparse_buffer);
+EXTRA_EXT(ARB_spirv_extensions);

static const int
extra_ARB_color_buffer_float_or_glcore[] = {
@@ -1151,6 +1153,11 @@ find_custom_value(struct gl_context *ctx, const struct value_desc *d, union valu
}
}
break;
+
+ /* ARB_spirv_extensions */
+ case GL_NUM_SPIR_V_EXTENSIONS:
+ v->value_int = _mesa_get_spirv_extension_count(ctx);
+ break;
}
}

diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py
index 20ef6e4977a..e2e803fba02 100644
--- a/src/mesa/main/get_hash_params.py
+++ b/src/mesa/main/get_hash_params.py
@@ -378,6 +378,9 @@ descriptor=[
# GL_ARB_sampler_objects / GL 3.3 / GLES 3.0
[ "SAMPLER_BINDING", "LOC_CUSTOM, TYPE_INT, GL_SAMPLER_BINDING, NO_EXTRA" ],

+# GL_ARB_spirv_extensions
+ [ "NUM_SPIR_V_EXTENSIONS", "LOC_CUSTOM, TYPE_INT, 0, extra_ARB_spirv_extensions" ],
+
# GL_ARB_sync
[ "MAX_SERVER_WAIT_TIMEOUT", "CONTEXT_INT64(Const.MaxServerWaitTimeout), extra_ARB_sync" ],

diff --git a/src/mesa/main/getstring.c b/src/mesa/main/getstring.c
index 931f6a476cb..23828aaf1b5 100644
--- a/src/mesa/main/getstring.c
+++ b/src/mesa/main/getstring.c
@@ -32,6 +32,7 @@
#include "extensions.h"
#include "mtypes.h"
#include "macros.h"
+#include "spirv_extensions.h"

/**
* Return the string for a glGetString(GL_SHADING_LANGUAGE_VERSION) query.
@@ -186,6 +187,17 @@ _mesa_GetStringi(GLenum name, GLuint index)
return (const GLubyte *) 0;
}
return _mesa_get_enabled_extension(ctx, index);
+ case GL_SPIR_V_EXTENSIONS:
+ if (!ctx->Extensions.ARB_spirv_extensions) {
+ _mesa_error(ctx, GL_INVALID_ENUM, "glGetStringi");
+ return (const GLubyte *) 0;
+ }
+
+ if (index >= _mesa_get_spirv_extension_count(ctx)) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glGetStringi(index=%u)", index);
+ return (const GLubyte *) 0;
+ }
+ return _mesa_get_enabled_spirv_extension(ctx, index);
default:
_mesa_error(ctx, GL_INVALID_ENUM, "glGetStringi");
return (const GLubyte *) 0;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 7fed85a2ae6..c63a5a67582 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4120,6 +4120,7 @@ struct gl_extensions
GLboolean ARB_shadow;
GLboolean ARB_sparse_buffer;
GLboolean ARB_stencil_texturing;
+ GLboolean ARB_spirv_extensions;
GLboolean ARB_sync;
GLboolean ARB_tessellation_shader;
GLboolean ARB_texture_border_clamp;
diff --git a/src/mesa/main/spirv_extensions.c b/src/mesa/main/spirv_extensions.c
new file mode 100644
index 00000000000..40a89c133aa
--- /dev/null
+++ b/src/mesa/main/spirv_extensions.c
@@ -0,0 +1,42 @@
+/*
+ * 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
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, 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 NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHOR(S) AND/OR THEIR SUPPLIERS 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.
+ */
+
+/**
+ * \file
+ * \brief SPIRV-V extension handling. See ARB_spirv_extensions
+ */
+
+#include "spirv_extensions.h"
+
+GLuint
+_mesa_get_spirv_extension_count(struct gl_context *ctx)
+{
+ return 0;
+}
+
+const GLubyte *
+_mesa_get_enabled_spirv_extension(struct gl_context *ctx,
+ GLuint index)
+{
+ return (const GLubyte *) 0;
+}
diff --git a/src/mesa/main/spirv_extensions.h b/src/mesa/main/spirv_extensions.h
new file mode 100644
index 00000000000..35754f7e53b
--- /dev/null
+++ b/src/mesa/main/spirv_extensions.h
@@ -0,0 +1,49 @@
+/*
+ * 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
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, 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 NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHOR(S) AND/OR THEIR SUPPLIERS 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.
+ */
+
+/**
+ * \file
+ * \brief SPIRV-V extension handling. See ARB_spirv_extensions
+ */
+
+#ifndef _SPIRVEXTENSIONS_H_
+#define _SPIRVEXTENSIONS_H_
+
+#include "mtypes.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern GLuint
+_mesa_get_spirv_extension_count(struct gl_context *ctx);
+
+extern const GLubyte *
+_mesa_get_enabled_spirv_extension(struct gl_context *ctx,
+ GLuint index);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* SPIRVEXTENSIONS_H */
diff --git a/src/mesa/meson.build b/src/mesa/meson.build
index 2eec7d45f7d..3d8548eb107 100644
--- a/src/mesa/meson.build
+++ b/src/mesa/meson.build
@@ -245,6 +245,8 @@ files_libmesa_common = files(
'main/shader_query.cpp',
'main/shared.c',
'main/shared.h',
+ 'main/spirv_extensions.c',
+ 'main/spirv_extensions.h',
'main/state.c',
'main/state.h',
'main/stencil.c',

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

Ideally this should be generated somehow. One option would be gather
all the extension dependencies listed on the core grammar, but there
would be the possibility of not including some of the extensions.

Note that spirv-tools is doing it just slightly better, as it has a
hardcoded list of extensions manually took from the registry, that
they parse to get the enum and the to_string method (see
generate_grammar_tables.py).

v2:
* Use a macro to improve readability. (Tapani Pälli)
* Add unreachable on the switch, no default (Eric Engestrom)
* No typedef enum (Ian Romanick)
* Sort extensions names (Ian Romanick)
* Don't add extensions unlikely to be supported by Mesa at any point
(Ian Romanick)
---
src/compiler/Makefile.sources | 2 ++
src/compiler/spirv/spirv_extensions.c | 46 +++++++++++++++++++++++++++++++
src/compiler/spirv/spirv_extensions.h | 51 +++++++++++++++++++++++++++++++++++
3 files changed, 99 insertions(+)
create mode 100644 src/compiler/spirv/spirv_extensions.c
create mode 100644 src/compiler/spirv/spirv_extensions.h

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index 2ab8e163a26..f198456c751 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -293,6 +293,8 @@ SPIRV_FILES = \
spirv/GLSL.std.450.h \
spirv/nir_spirv.h \
spirv/spirv.h \
+ spirv/spirv_extensions.c \
+ spirv/spirv_extensions.h \
spirv/spirv_info.h \
spirv/spirv_to_nir.c \
spirv/vtn_alu.c \
diff --git a/src/compiler/spirv/spirv_extensions.c b/src/compiler/spirv/spirv_extensions.c
new file mode 100644
index 00000000000..f50f87b52e1
--- /dev/null
+++ b/src/compiler/spirv/spirv_extensions.c
@@ -0,0 +1,46 @@
+/*
+ * 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 "spirv.h"
+#include "spirv_extensions.h"
+
+const char *
+spirv_extensions_to_string(enum SpvExtension ext)
+{
+#define STR(x) case x: return #x;
+ switch (ext) {
+ STR(SPV_KHR_16bit_storage);
+ STR(SPV_KHR_device_group);
+ STR(SPV_KHR_multiview);
+ STR(SPV_KHR_shader_ballot);
+ STR(SPV_KHR_shader_draw_parameters);
+ STR(SPV_KHR_storage_buffer_storage_class);
+ STR(SPV_KHR_subgroup_vote);
+ STR(SPV_KHR_variable_pointers);
+ case SPV_EXTENSIONS_COUNT:
+ unreachable("Unknown SPIR-V extension");
+ }
+#undef STR
+
+ return "unknown";
+}
diff --git a/src/compiler/spirv/spirv_extensions.h b/src/compiler/spirv/spirv_extensions.h
new file mode 100644
index 00000000000..0568132a517
--- /dev/null
+++ b/src/compiler/spirv/spirv_extensions.h
@@ -0,0 +1,51 @@
+/*
+ * 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.
+ */
+
+#ifndef _SPIRV_EXTENSIONS_H_
+#define _SPIRV_EXTENSIONS_H_
+
+#include "compiler/nir/nir.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+enum SpvExtension {
+ SPV_KHR_16bit_storage = 0,
+ SPV_KHR_device_group,
+ SPV_KHR_multiview,
+ SPV_KHR_shader_ballot,
+ SPV_KHR_shader_draw_parameters,
+ SPV_KHR_storage_buffer_storage_class,
+ SPV_KHR_subgroup_vote,
+ SPV_KHR_variable_pointers,
+ SPV_EXTENSIONS_COUNT
+};
+
+const char *spirv_extensions_to_string(enum SpvExtension ext);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* SPIRV_EXTENSIONS */

[Timothy Arceri]

I might be blind but in which patch does this end up being used?

Also this and the following patches seem to reinvent a system for
exposing extension support. Is there a good reason for not simply
expanding extensions_table.h to support tracking the spriv extensions?

Post by Eduardo Lima Mitev
Ideally this should be generated somehow. One option would be gather
all the extension dependencies listed on the core grammar, but there
would be the possibility of not including some of the extensions.
Note that spirv-tools is doing it just slightly better, as it has a
hardcoded list of extensions manually took from the registry, that
they parse to get the enum and the to_string method (see
generate_grammar_tables.py).
* Use a macro to improve readability. (Tapani Pälli)
* Add unreachable on the switch, no default (Eric Engestrom)
* No typedef enum (Ian Romanick)
* Sort extensions names (Ian Romanick)
* Don't add extensions unlikely to be supported by Mesa at any point
(Ian Romanick)
---
src/compiler/Makefile.sources | 2 ++
src/compiler/spirv/spirv_extensions.c | 46 +++++++++++++++++++++++++++++++
src/compiler/spirv/spirv_extensions.h | 51 +++++++++++++++++++++++++++++++++++
3 files changed, 99 insertions(+)
create mode 100644 src/compiler/spirv/spirv_extensions.c
create mode 100644 src/compiler/spirv/spirv_extensions.h
diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index 2ab8e163a26..f198456c751 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -293,6 +293,8 @@ SPIRV_FILES = \
spirv/GLSL.std.450.h \
spirv/nir_spirv.h \
spirv/spirv.h \
+ spirv/spirv_extensions.c \
+ spirv/spirv_extensions.h \
spirv/spirv_info.h \
spirv/spirv_to_nir.c \
spirv/vtn_alu.c \
diff --git a/src/compiler/spirv/spirv_extensions.c b/src/compiler/spirv/spirv_extensions.c
new file mode 100644
index 00000000000..f50f87b52e1
--- /dev/null
+++ b/src/compiler/spirv/spirv_extensions.c
@@ -0,0 +1,46 @@
+/*
+ * 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
+ *
+ * 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.h"
+#include "spirv_extensions.h"
+
+const char *
+spirv_extensions_to_string(enum SpvExtension ext)
+{
+#define STR(x) case x: return #x;
+ switch (ext) {
+ STR(SPV_KHR_16bit_storage);
+ STR(SPV_KHR_device_group);
+ STR(SPV_KHR_multiview);
+ STR(SPV_KHR_shader_ballot);
+ STR(SPV_KHR_shader_draw_parameters);
+ STR(SPV_KHR_storage_buffer_storage_class);
+ STR(SPV_KHR_subgroup_vote);
+ STR(SPV_KHR_variable_pointers);
+ unreachable("Unknown SPIR-V extension");
+ }
+#undef STR
+
+ return "unknown";
+}
diff --git a/src/compiler/spirv/spirv_extensions.h b/src/compiler/spirv/spirv_extensions.h
new file mode 100644
index 00000000000..0568132a517
--- /dev/null
+++ b/src/compiler/spirv/spirv_extensions.h
@@ -0,0 +1,51 @@
+/*
+ * 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
+ *
+ * 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_EXTENSIONS_H_
+#define _SPIRV_EXTENSIONS_H_
+
+#include "compiler/nir/nir.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+enum SpvExtension {
+ SPV_KHR_16bit_storage = 0,
+ SPV_KHR_device_group,
+ SPV_KHR_multiview,
+ SPV_KHR_shader_ballot,
+ SPV_KHR_shader_draw_parameters,
+ SPV_KHR_storage_buffer_storage_class,
+ SPV_KHR_subgroup_vote,
+ SPV_KHR_variable_pointers,
+ SPV_EXTENSIONS_COUNT
+};
+
+const char *spirv_extensions_to_string(enum SpvExtension ext);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* SPIRV_EXTENSIONS */

[Alejandro Piñeiro]

Post by Timothy Arceri
I might be blind but in which patch does this end up being used?

On the patch "spirv_extensions: add spirv_supported_extensions on
gl_constants" is used. That patch adds spirv_supported_extensions on
gl_constants, and as the commit message says, uses it at the
implementation of _mesa_get_spirv_extension_count and
_mesa_get_enabled_spirv_extension to get real values. The patch
"spirv_extensions: i965: initialize SPIR-V extensions" fill it up for i965.

Post by Timothy Arceri
Also this and the following patches seem to reinvent a system for
exposing extension support. Is there a good reason for not simply
expanding extensions_table.h to support tracking the spriv extensions?

They are a different set of extensions that are enabled or not in a
different way. On the case of opengl extensions they are enabled
individually by each driver. spirv extensions are enabled based on spirv
capabilities. And spirv extensions tracking doesn't need so many
features as tracking opengl extensions. We could make it more similar to
extensions_table.h, but at the end of the day, it would be two different
implementations with different needs. I just focused in keeping it simple.

BR

Post by Timothy Arceri

Post by Eduardo Lima Mitev
Ideally this should be generated somehow. One option would be gather
all the extension dependencies listed on the core grammar, but there
would be the possibility of not including some of the extensions.
Note that spirv-tools is doing it just slightly better, as it has a
hardcoded list of extensions manually took from the registry, that
they parse to get the enum and the to_string method (see
generate_grammar_tables.py).
   * Use a macro to improve readability. (Tapani Pälli)
   * Add unreachable on the switch, no default (Eric Engestrom)
   * No typedef enum (Ian Romanick)
   * Sort extensions names (Ian Romanick)
   * Don't add extensions unlikely to be supported by Mesa at any point
     (Ian Romanick)
---
  src/compiler/Makefile.sources         |  2 ++
  src/compiler/spirv/spirv_extensions.c | 46
+++++++++++++++++++++++++++++++
  src/compiler/spirv/spirv_extensions.h | 51
+++++++++++++++++++++++++++++++++++
  3 files changed, 99 insertions(+)
  create mode 100644 src/compiler/spirv/spirv_extensions.c
  create mode 100644 src/compiler/spirv/spirv_extensions.h
diff --git a/src/compiler/Makefile.sources
b/src/compiler/Makefile.sources
index 2ab8e163a26..f198456c751 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -293,6 +293,8 @@ SPIRV_FILES = \
      spirv/GLSL.std.450.h \
      spirv/nir_spirv.h \
      spirv/spirv.h \
+    spirv/spirv_extensions.c \
+    spirv/spirv_extensions.h \
      spirv/spirv_info.h \
      spirv/spirv_to_nir.c \
      spirv/vtn_alu.c \
diff --git a/src/compiler/spirv/spirv_extensions.c
b/src/compiler/spirv/spirv_extensions.c
new file mode 100644
index 00000000000..f50f87b52e1
--- /dev/null
+++ b/src/compiler/spirv/spirv_extensions.c
@@ -0,0 +1,46 @@
+/*
+ * 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
+ *
+ * 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.h"
+#include "spirv_extensions.h"
+
+const char *
+spirv_extensions_to_string(enum SpvExtension ext)
+{
+#define STR(x) case x: return #x;
+   switch (ext) {
+   STR(SPV_KHR_16bit_storage);
+   STR(SPV_KHR_device_group);
+   STR(SPV_KHR_multiview);
+   STR(SPV_KHR_shader_ballot);
+   STR(SPV_KHR_shader_draw_parameters);
+   STR(SPV_KHR_storage_buffer_storage_class);
+   STR(SPV_KHR_subgroup_vote);
+   STR(SPV_KHR_variable_pointers);
+      unreachable("Unknown SPIR-V extension");
+   }
+#undef STR
+
+   return "unknown";
+}
diff --git a/src/compiler/spirv/spirv_extensions.h
b/src/compiler/spirv/spirv_extensions.h
new file mode 100644
index 00000000000..0568132a517
--- /dev/null
+++ b/src/compiler/spirv/spirv_extensions.h
@@ -0,0 +1,51 @@
+/*
+ * 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
+ *
+ * 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_EXTENSIONS_H_
+#define _SPIRV_EXTENSIONS_H_
+
+#include "compiler/nir/nir.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+enum SpvExtension {
+   SPV_KHR_16bit_storage = 0,
+   SPV_KHR_device_group,
+   SPV_KHR_multiview,
+   SPV_KHR_shader_ballot,
+   SPV_KHR_shader_draw_parameters,
+   SPV_KHR_storage_buffer_storage_class,
+   SPV_KHR_subgroup_vote,
+   SPV_KHR_variable_pointers,
+   SPV_EXTENSIONS_COUNT
+};
+
+const char *spirv_extensions_to_string(enum SpvExtension ext);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* SPIRV_EXTENSIONS */

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

Add a struct to maintain which SPIR-V extensions are supported, and an
utility method to initialize it based on
nir_spirv_supported_capabilities.

v2:
* Fixing code style (Ian Romanick)
* Adding a prefix (spirv) to fill_supported_spirv_extensions (Ian Romanick)
---
src/compiler/spirv/spirv_extensions.c | 31 +++++++++++++++++++++++++++++++
src/compiler/spirv/spirv_extensions.h | 12 ++++++++++++
2 files changed, 43 insertions(+)

diff --git a/src/compiler/spirv/spirv_extensions.c b/src/compiler/spirv/spirv_extensions.c
index f50f87b52e1..ae0ceec9a47 100644
--- a/src/compiler/spirv/spirv_extensions.c
+++ b/src/compiler/spirv/spirv_extensions.c
@@ -44,3 +44,34 @@ spirv_extensions_to_string(enum SpvExtension ext)

return "unknown";
}
+
+/**
+ * Sets the supported flags for known SPIR-V extensions based on the
+ * capabilites supported (spirv capabilities based on the spirv to nir
+ * support).
+ *
+ * One could argue that makes more sense in the other way around, as from the
+ * spec pov capabilities are enable for a given extension. But from our pov,
+ * we support or not (depending on the driver) some given capability, and
+ * spirv_to_nir check for capabilities not extensions. Also we usually fill
+ * first the supported capabilities, that are not always related to an
+ * extension.
+ */
+void
+spirv_fill_supported_spirv_extensions(struct spirv_supported_extensions *ext,
+ const struct nir_spirv_supported_capabilities *cap)
+{
+ for (unsigned i = 0; i < SPV_EXTENSIONS_COUNT; i++)
+ ext->supported[i] = false;
+
+ ext->count = 0;
+
+ ext->supported[SPV_KHR_shader_draw_parameters] = cap->draw_parameters;
+ ext->supported[SPV_KHR_multiview] = cap->multiview;
+ ext->supported[SPV_KHR_variable_pointers] = cap->variable_pointers;
+
+ for (unsigned i = 0; i < SPV_EXTENSIONS_COUNT; i++) {
+ if (ext->supported[i])
+ ext->count++;
+ }
+}
diff --git a/src/compiler/spirv/spirv_extensions.h b/src/compiler/spirv/spirv_extensions.h
index 0568132a517..7c3d9f0f563 100644
--- a/src/compiler/spirv/spirv_extensions.h
+++ b/src/compiler/spirv/spirv_extensions.h
@@ -25,6 +25,7 @@
#define _SPIRV_EXTENSIONS_H_

#include "compiler/nir/nir.h"
+#include "nir_spirv.h"

#ifdef __cplusplus
extern "C" {
@@ -42,8 +43,19 @@ enum SpvExtension {
SPV_EXTENSIONS_COUNT
};

+struct spirv_supported_extensions {
+ /** Flags the supported extensions. Array to make it easier to iterate. */
+ bool supported[SPV_EXTENSIONS_COUNT];
+
+ /** Number of supported extensions */
+ unsigned int count;
+};
+
const char *spirv_extensions_to_string(enum SpvExtension ext);

+void spirv_fill_supported_spirv_extensions(struct spirv_supported_extensions *ext,
+ const struct nir_spirv_supported_capabilities *cap);
+
#ifdef __cplusplus
}
#endif

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

We can use it to get real values for ARB_spirv_extensions methods.

v2: Rebase update after changes on previous patches.
---
src/mesa/main/mtypes.h | 3 +++
src/mesa/main/spirv_extensions.c | 20 +++++++++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index c63a5a67582..d74bf10daa0 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4031,6 +4031,9 @@ struct gl_constants

/** GL_ARB_gl_spirv */
struct nir_spirv_supported_capabilities SpirVCapabilities;
+
+ /** GL_ARB_spirv_extensions */
+ struct spirv_supported_extensions *SpirVExtensions;
};


diff --git a/src/mesa/main/spirv_extensions.c b/src/mesa/main/spirv_extensions.c
index 40a89c133aa..2bb29461fd4 100644
--- a/src/mesa/main/spirv_extensions.c
+++ b/src/mesa/main/spirv_extensions.c
@@ -27,16 +27,34 @@
*/

#include "spirv_extensions.h"
+#include "compiler/spirv/spirv_extensions.h"

GLuint
_mesa_get_spirv_extension_count(struct gl_context *ctx)
{
- return 0;
+ if (ctx->Const.SpirVExtensions == NULL)
+ return 0;
+
+ return ctx->Const.SpirVExtensions->count;
}

const GLubyte *
_mesa_get_enabled_spirv_extension(struct gl_context *ctx,
GLuint index)
{
+ unsigned int n = 0;
+
+ if (ctx->Const.SpirVExtensions == NULL)
+ return (const GLubyte *) 0;
+
+ for (unsigned int i = 0; i < SPV_EXTENSIONS_COUNT; i++) {
+ if (ctx->Const.SpirVExtensions->supported[i]) {
+ if (n == index)
+ return (const GLubyte *) spirv_extensions_to_string(i);
+ else
+ n++;
+ }
+ }
+
return (const GLubyte *) 0;
}

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

v2: Rebase update after changes on previous patches.
---
src/mesa/drivers/dri/i965/brw_context.c | 6 ++++++
src/mesa/main/context.c | 2 ++
2 files changed, 8 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index d90b7797a7a..6afc18bcf6e 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -76,6 +76,7 @@
#include "isl/isl.h"

#include "compiler/spirv/nir_spirv.h"
+#include "compiler/spirv/spirv_extensions.h"
/***************************************
* Mesa's Driver Functions
***************************************/
@@ -716,6 +717,11 @@ brw_initialize_context_constants(struct brw_context *brw)
/* GL_ARB_gl_spirv */
if (ctx->Version >= 33)
brw_initialize_spirv_supported_capabilities(brw);
+
+ /* GL_ARB_spirv_extensions */
+ ctx->Const.SpirVExtensions = MALLOC_STRUCT(spirv_supported_extensions);
+ spirv_fill_supported_spirv_extensions(ctx->Const.SpirVExtensions,
+ &ctx->Const.SpirVCapabilities);
}

static void
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 3fa9f69f883..b48481d4372 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1377,6 +1377,8 @@ _mesa_free_context_data( struct gl_context *ctx )
if (ctx == _mesa_get_current_context()) {
_mesa_make_current(NULL, NULL, NULL);
}
+
+ free(ctx->Const.SpirVExtensions);
}

[Eduardo Lima Mitev]

From: Alejandro Piñeiro <***@igalia.com>

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
---
src/compiler/spirv/nir_spirv.h | 6 ++
src/compiler/spirv/spirv_to_nir.c | 156 +++++++++++++++++++++++++++++++++++---
2 files changed, 152 insertions(+), 10 deletions(-)

diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index a14b55cdd4b..8b8942fc726 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -41,8 +41,14 @@ struct nir_spirv_specialization {
uint32_t data32;
uint64_t data64;
};
+ bool defined_on_module;
};

+
+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 6034228ed36..fea3ad7b09d 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -1031,6 +1031,7 @@ spec_constant_decoration_cb(struct vtn_builder *b, struct vtn_value *v,
const_value->data64 = b->specializations[i].data64;
else
const_value->data32 = b->specializations[i].data32;
+ b->specializations[i].defined_on_module = true;
return;
}
}
@@ -1065,6 +1066,11 @@ handle_workgroup_size_decoration_cb(struct vtn_builder *b,
const struct vtn_decoration *dec,
void *data)
{
+ /* This can happens if we are gl_spirv_validation. We can return safely, as
+ * we don't need the workgroup info for such validation. */
+ if (b->shader == NULL)
+ return;
+
assert(member == -1);
if (dec->decoration != SpvDecorationBuiltIn ||
dec->literals[0] != SpvBuiltInWorkgroupSize)
@@ -2848,6 +2854,49 @@ vtn_handle_preamble_instruction(struct vtn_builder *b, SpvOp opcode,
return true;
}

+/*
+ * gl_spirv validation. Just need to check for the entry point.
+ */
+static bool
+vtn_validate_preamble_instruction(struct vtn_builder *b, SpvOp opcode,
+ const uint32_t *w, unsigned count)
+{
+ switch (opcode) {
+ /* The following opcodes are not needed for gl_spirv, so we can skip
+ * them.
+ */
+ 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_preamble_instruction(b, opcode, w, count);
+ break;
+
+ case SpvOpDecorate:
+ vtn_handle_decoration(b, opcode, w, count);
+ break;
+
+ default:
+ return false; /* End of preamble */
+ }
+
+ return true;
+}
+
static void
vtn_handle_execution_mode(struct vtn_builder *b, struct vtn_value *entry_point,
const struct vtn_decoration *mode, void *data)
@@ -3055,6 +3104,22 @@ vtn_handle_variable_or_type_instruction(struct vtn_builder *b, SpvOp opcode,
return true;
}

+static bool
+vtn_handle_constant_or_type_instruction(struct vtn_builder *b, SpvOp opcode,
+ const uint32_t *w, unsigned count)
+{
+ switch (opcode) {
+ case SpvOpUndef:
+ case SpvOpVariable:
+ break;
+
+ default:
+ return vtn_handle_variable_or_type_instruction(b, opcode, w, count);
+ }
+
+ return true;
+}
+
static bool
vtn_handle_body_instruction(struct vtn_builder *b, SpvOp opcode,
const uint32_t *w, unsigned count)
@@ -3309,15 +3374,10 @@ vtn_handle_body_instruction(struct vtn_builder *b, SpvOp opcode,
return true;
}

-nir_function *
-spirv_to_nir(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,
- const struct nir_spirv_supported_capabilities *cap,
- const nir_shader_compiler_options *options)
+static struct vtn_builder*
+common_initialization(const uint32_t *words, size_t word_count,
+ gl_shader_stage stage, const char *entry_point_name)
{
- const uint32_t *word_end = words + word_count;
-
/* Handle the SPIR-V header (first 4 dwords) */
assert(word_count > 5);

@@ -3327,8 +3387,6 @@ spirv_to_nir(const uint32_t *words, size_t word_count,
unsigned value_id_bound = words[3];
assert(words[4] == 0);

- words+= 5;
-
/* Initialize the stn_builder object */
struct vtn_builder *b = rzalloc(NULL, struct vtn_builder);
b->value_id_bound = value_id_bound;
@@ -3336,6 +3394,84 @@ spirv_to_nir(const uint32_t *words, size_t word_count,
exec_list_make_empty(&b->functions);
b->entry_point_stage = stage;
b->entry_point_name = entry_point_name;
+
+ return b;
+}
+
+/*
+ * 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* pConstantVaulue);
+ * <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)
+{
+ const uint32_t *word_end = words + word_count;
+
+ struct vtn_builder *b = common_initialization(words, word_count,
+ stage, entry_point_name);
+ 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 type, and constant instructions (we don't need to handle
+ * variables for gl_spirv)
+ */
+ words = vtn_foreach_instruction(b, words, word_end,
+ vtn_handle_constant_or_type_instruction);
+
+ ralloc_free(b);
+
+ return true;
+}
+
+nir_function *
+spirv_to_nir(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,
+ const struct nir_spirv_supported_capabilities *cap,
+ const nir_shader_compiler_options *options)
+{
+ const uint32_t *word_end = words + word_count;
+
+ struct vtn_builder *b = common_initialization(words, word_count,
+ stage, entry_point_name);
+ words+= 5;
b->cap = cap;

/* Handle all the preamble instructions */

[Eduardo Lima Mitev]

From: Nicolai Hähnle <***@amd.com>

v2: * Use gl_spirv_validation instead of spirv_to_nir.
This method just validates the shader. The conversion to NIR will
happen later, during linking. (Alejandro Piñeiro)

* Use gl_shader_spirv_data struct to store the SPIR-V data.
(Eduardo Lima)

* Use the 'spirv_data' member to tell if the gl_shader is
a SPIR-V shader, instead of a dedicated flag. (Timothy Arceri)
---
src/mesa/main/glspirv.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 103 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index d2e76bb1927..18710c0d8fc 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -24,8 +24,10 @@
#include "glspirv.h"

#include "errors.h"
+#include "shaderobj.h"

-#include "errors.h"
+#include "compiler/nir/nir.h"
+#include "compiler/spirv/nir_spirv.h"

#include "util/u_atomic.h"

@@ -110,7 +112,105 @@ _mesa_SpecializeShaderARB(GLuint shader,
const GLuint *pConstantValue)
{
GET_CURRENT_CONTEXT(ctx);
+ struct gl_shader *sh;
+ bool has_entry_point;
+ struct nir_spirv_specialization *spec_entries = NULL;
+
+ if (!ctx->Extensions.ARB_gl_spirv) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "glSpecializeShaderARB");
+ return;
+ }
+
+ sh = _mesa_lookup_shader_err(ctx, shader, "glSpecializeShaderARB");
+ if (!sh)
+ return;
+
+ if (!sh->spirv_data) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glSpecializeShaderARB(not SPIR-V)");
+ return;
+ }
+
+ if (sh->CompileStatus) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "glSpecializeShaderARB(already specialized)");
+ return;
+ }
+
+ struct gl_shader_spirv_data *spirv_data = sh->spirv_data;
+
+ /* From the GL_ARB_gl_spirv spec:
+ *
+ * "The OpenGL API expects the SPIR-V module to have already been
+ * validated, and can return an error if it discovers anything invalid
+ * in the module. An invalid SPIR-V module is allowed to result in
+ * undefined behavior."
+ *
+ * However, the following errors still need to be detected (from the same
+ * spec):
+ *
+ * "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>."
+ *
+ * We cannot flag those errors a-priori because detecting them requires
+ * parsing the module. However, flagging them during specialization is okay,
+ * since it makes no difference in terms of application-visible state.
+ */
+ spec_entries = calloc(sizeof(*spec_entries), numSpecializationConstants);
+
+ for (unsigned i = 0; i < numSpecializationConstants; ++i) {
+ spec_entries[i].id = pConstantIndex[i];
+ spec_entries[i].data32 = pConstantValue[i];
+ spec_entries[i].defined_on_module = false;
+ }
+
+ has_entry_point =
+ gl_spirv_validation((uint32_t *)&spirv_data->SpirVModule->Binary[0],
+ spirv_data->SpirVModule->Length / 4,
+ spec_entries, numSpecializationConstants,
+ sh->Stage, pEntryPoint);
+
+ /* See previous spec comment */
+ if (!has_entry_point) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glSpecializeShaderARB(\"%s\" is not a valid entry point"
+ " for shader)", pEntryPoint);
+ goto end;
+ }
+
+ for (unsigned i = 0; i < numSpecializationConstants; ++i) {
+ if (spec_entries[i].defined_on_module == false) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glSpecializeShaderARB(constant \"%i\" does not exist "
+ "in shader)", spec_entries[i].id);
+ goto end;
+ }
+ }
+
+ spirv_data->SpirVEntryPoint = ralloc_strdup(spirv_data, pEntryPoint);
+
+ /* Note that we didn't make a real compilation of the module (spirv_to_nir),
+ * but just checked some error conditions. Real "compilation" will be done
+ * later, upon linking.
+ */
+ sh->CompileStatus = compile_success;
+
+ spirv_data->NumSpecializationConstants = numSpecializationConstants;
+ spirv_data->SpecializationConstantsIndex =
+ rzalloc_array_size(spirv_data, sizeof(GLuint),
+ numSpecializationConstants);
+ spirv_data->SpecializationConstantsValue =
+ rzalloc_array_size(spirv_data, sizeof(GLuint),
+ numSpecializationConstants);
+ for (unsigned i = 0; i < numSpecializationConstants; ++i) {
+ spirv_data->SpecializationConstantsIndex[i] = pConstantIndex[i];
+ spirv_data->SpecializationConstantsValue[i] = pConstantValue[i];
+ }

- /* Just return GL_INVALID_OPERATION error while this is boilerplate */
- _mesa_error(ctx, GL_INVALID_OPERATION, "SpecializeShaderARB");
+ end:
+ free(spec_entries);
}

[Eduardo Lima Mitev]

This will be the equivalent to link_shaders() from
src/compiler/glsl/linker.cpp, but for SPIR-V programs.
---
src/mesa/main/glspirv.c | 10 ++++++++++
src/mesa/main/glspirv.h | 4 ++++
2 files changed, 14 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 18710c0d8fc..e533853f7fa 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -104,6 +104,16 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
}

+void
+_mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
+{
+ /* @TODO: This is a placeholder for the equivalent of
+ * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
+ */
+ prog->data->LinkStatus = linking_success;
+ prog->data->Validated = false;
+}
+
void GLAPIENTRY
_mesa_SpecializeShaderARB(GLuint shader,
const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index ba281f68bef..0f03b75c111 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -76,6 +76,10 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
unsigned n, struct gl_shader **shaders,
const void* binary, size_t length);

+void
+_mesa_spirv_link_shaders(struct gl_context *ctx,
+ struct gl_shader_program *prog);
+
/**
* \name API functions
*/

[Timothy Arceri]

Please squash this with patch 22 tis is just code churn.

Post by Eduardo Lima Mitev
This will be the equivalent to link_shaders() from
src/compiler/glsl/linker.cpp, but for SPIR-V programs.
---
src/mesa/main/glspirv.c | 10 ++++++++++
src/mesa/main/glspirv.h | 4 ++++
2 files changed, 14 insertions(+)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 18710c0d8fc..e533853f7fa 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -104,6 +104,16 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
}
+void
+_mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
+{
+ * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
+ */
+ prog->data->LinkStatus = linking_success;
+ prog->data->Validated = false;
+}
+
void GLAPIENTRY
_mesa_SpecializeShaderARB(GLuint shader,
const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index ba281f68bef..0f03b75c111 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -76,6 +76,10 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
unsigned n, struct gl_shader **shaders,
const void* binary, size_t length);
+void
+_mesa_spirv_link_shaders(struct gl_context *ctx,
+ struct gl_shader_program *prog);
+
/**
* \name API functions
*/

[Eduardo Lima Mitev]

Post by Timothy Arceri
Please squash this with patch 22 tis is just code churn.

Ok, this makes sense, though I will have to re-order a bit the patches
to keep each single one building fine.

I the squashed result would keep your R-b from patch 22, right?

thnaks,
Eduardo

Post by Timothy Arceri

Post by Eduardo Lima Mitev
This will be the equivalent to link_shaders() from
src/compiler/glsl/linker.cpp, but for SPIR-V programs.
---
  src/mesa/main/glspirv.c | 10 ++++++++++
  src/mesa/main/glspirv.h |  4 ++++
  2 files changed, 14 insertions(+)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 18710c0d8fc..e533853f7fa 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -104,6 +104,16 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
     }
  }
  +void
+_mesa_spirv_link_shaders(struct gl_context *ctx, struct
gl_shader_program *prog)
+{
+    * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
+    */
+   prog->data->LinkStatus = linking_success;
+   prog->data->Validated = false;
+}
+
  void GLAPIENTRY
  _mesa_SpecializeShaderARB(GLuint shader,
                            const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index ba281f68bef..0f03b75c111 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -76,6 +76,10 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
                            unsigned n, struct gl_shader **shaders,
                            const void* binary, size_t length);
  +void
+_mesa_spirv_link_shaders(struct gl_context *ctx,
+                         struct gl_shader_program *prog);
+
  /**
   * \name API functions
   */

[Timothy Arceri]

Post by Eduardo Lima Mitev

Post by Timothy Arceri
Please squash this with patch 22 tis is just code churn.

Ok, this makes sense, though I will have to re-order a bit the patches
to keep each single one building fine.

None of the patches in between seem to use it so it should be fine

Post by Eduardo Lima Mitev
I the squashed result would keep your R-b from patch 22, right?

Correct :)

Post by Eduardo Lima Mitev
thnaks,
Eduardo

Post by Timothy Arceri

Post by Eduardo Lima Mitev
This will be the equivalent to link_shaders() from
src/compiler/glsl/linker.cpp, but for SPIR-V programs.
---
  src/mesa/main/glspirv.c | 10 ++++++++++
  src/mesa/main/glspirv.h |  4 ++++
  2 files changed, 14 insertions(+)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 18710c0d8fc..e533853f7fa 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -104,6 +104,16 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
     }
  }
  +void
+_mesa_spirv_link_shaders(struct gl_context *ctx, struct
gl_shader_program *prog)
+{
+    * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
+    */
+   prog->data->LinkStatus = linking_success;
+   prog->data->Validated = false;
+}
+
  void GLAPIENTRY
  _mesa_SpecializeShaderARB(GLuint shader,
                            const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index ba281f68bef..0f03b75c111 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -76,6 +76,10 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
                            unsigned n, struct gl_shader **shaders,
                            const void* binary, size_t length);
  +void
+_mesa_spirv_link_shaders(struct gl_context *ctx,
+                         struct gl_shader_program *prog);
+
  /**
   * \name API functions
   */

[Eduardo Lima Mitev]

---
src/mesa/program/ir_to_mesa.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 047f5b38f71..83de0143c65 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -36,6 +36,7 @@
#include "main/shaderapi.h"
#include "main/shaderobj.h"
#include "main/uniforms.h"
+#include "main/glspirv.h"
#include "compiler/glsl/ast.h"
#include "compiler/glsl/ir.h"
#include "compiler/glsl/ir_expression_flattening.h"
@@ -3106,7 +3107,10 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
}

if (prog->data->LinkStatus) {
- link_shaders(ctx, prog);
+ if (!spirv)
+ link_shaders(ctx, prog);
+ else
+ _mesa_spirv_link_shaders(ctx, prog);
}

if (prog->data->LinkStatus) {

[Eduardo Lima Mitev]

This is a reference to the spirv_data object stored in gl_shader, which
stores shader SPIR-V data that is needed during linking too.
---
src/mesa/main/mtypes.h | 8 ++++++++
src/mesa/main/shaderobj.c | 1 +
2 files changed, 9 insertions(+)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index d74bf10daa0..1c8de9542e8 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2542,6 +2542,14 @@ struct gl_linked_shader
struct exec_list *packed_varyings;
struct exec_list *fragdata_arrays;
struct glsl_symbol_table *symbols;
+
+ /**
+ * ARB_gl_spirv related data.
+ *
+ * This is actually a reference to the gl_shader::spirv_data, which
+ * stores information that is also needed during linking.
+ */
+ struct gl_shader_spirv_data *spirv_data;
};


diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index 5c1cdd6b27a..834e2a92ec4 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -137,6 +137,7 @@ void
_mesa_delete_linked_shader(struct gl_context *ctx,
struct gl_linked_shader *sh)
{
+ _mesa_shader_spirv_data_reference(&sh->spirv_data, NULL);
_mesa_reference_program(ctx, &sh->Program, NULL);
ralloc_free(sh);
}

[Timothy Arceri]

Post by Eduardo Lima Mitev
This is a reference to the spirv_data object stored in gl_shader, which
stores shader SPIR-V data that is needed during linking too.
---
src/mesa/main/mtypes.h | 8 ++++++++
src/mesa/main/shaderobj.c | 1 +
2 files changed, 9 insertions(+)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index d74bf10daa0..1c8de9542e8 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2542,6 +2542,14 @@ struct gl_linked_shader
struct exec_list *packed_varyings;
struct exec_list *fragdata_arrays;
struct glsl_symbol_table *symbols;
+
+ /**
+ * ARB_gl_spirv related data.
+ *
+ * This is actually a reference to the gl_shader::spirv_data, which
+ * stores information that is also needed during linking.
+ */
+ struct gl_shader_spirv_data *spirv_data;
};
diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index 5c1cdd6b27a..834e2a92ec4 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -137,6 +137,7 @@ void
_mesa_delete_linked_shader(struct gl_context *ctx,
struct gl_linked_shader *sh)
{
+ _mesa_shader_spirv_data_reference(&sh->spirv_data, NULL);
_mesa_reference_program(ctx, &sh->Program, NULL);
ralloc_free(sh);
}

[Eduardo Lima Mitev]

---
src/mesa/main/glspirv.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e533853f7fa..e5dc8b26ea9 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -29,6 +29,8 @@
#include "compiler/nir/nir.h"
#include "compiler/spirv/nir_spirv.h"

+#include "program/program.h"
+
#include "util/u_atomic.h"

void
@@ -104,14 +106,57 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
}

+/**
+ * This is the equivalent to compiler/glsl/linker.cpp::link_shaders()
+ * but for SPIR-V programs.
+ *
+ * This method just creates the gl_linked_shader structs with a reference to
+ * the SPIR-V data collected during previous steps.
+ *
+ * The real linking happens later in the driver-specifc call LinkShader().
+ * This is so backends can implement different linking strategies for
+ * SPIR-V programs.
+ */
void
_mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
{
- /* @TODO: This is a placeholder for the equivalent of
- * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
- */
prog->data->LinkStatus = linking_success;
prog->data->Validated = false;
+
+ for (unsigned i = 0; i < prog->NumShaders; i++) {
+ struct gl_shader *shader = prog->Shaders[i];
+ gl_shader_stage shader_type = shader->Stage;
+
+ assert(shader->spirv_data);
+
+ struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader);
+ linked->Stage = shader_type;
+
+ /* Create program and attach it to the linked shader */
+ struct gl_program *gl_prog =
+ ctx->Driver.NewProgram(ctx,
+ _mesa_shader_stage_to_program(shader_type),
+ prog->Name, false);
+ if (!gl_prog) {
+ prog->data->LinkStatus = linking_failure;
+ _mesa_delete_linked_shader(ctx, linked);
+ return;
+ }
+
+ _mesa_reference_shader_program_data(ctx,
+ &gl_prog->sh.data,
+ prog->data);
+
+ /* Don't use _mesa_reference_program() just take ownership */
+ linked->Program = gl_prog;
+
+ /* Reference the SPIR-V data from shader to the linked shader */
+ _mesa_shader_spirv_data_reference(&linked->spirv_data,
+ shader->spirv_data);
+
+ prog->_LinkedShaders[shader_type] = linked;
+ prog->data->linked_stages |= 1 << shader_type;
+ }
}

void GLAPIENTRY

[Eduardo Lima Mitev]

v2: Bail out if we see more that one shader for the same stage, and add
a corresponding comment. (Timothy Arceri)
---
src/mesa/main/glspirv.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e533853f7fa..0934ceccbb3 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -29,6 +29,8 @@
#include "compiler/nir/nir.h"
#include "compiler/spirv/nir_spirv.h"

+#include "program/program.h"
+
#include "util/u_atomic.h"

void
@@ -104,14 +106,67 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
}

+/**
+ * This is the equivalent to compiler/glsl/linker.cpp::link_shaders()
+ * but for SPIR-V programs.
+ *
+ * This method just creates the gl_linked_shader structs with a reference to
+ * the SPIR-V data collected during previous steps.
+ *
+ * The real linking happens later in the driver-specifc call LinkShader().
+ * This is so backends can implement different linking strategies for
+ * SPIR-V programs.
+ */
void
_mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
{
- /* @TODO: This is a placeholder for the equivalent of
- * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
- */
prog->data->LinkStatus = linking_success;
prog->data->Validated = false;
+
+ for (unsigned i = 0; i < prog->NumShaders; i++) {
+ struct gl_shader *shader = prog->Shaders[i];
+ gl_shader_stage shader_type = shader->Stage;
+
+ /* We only support one shader per stage. The gl_spirv spec doesn't seem
+ * to prevent this, but the way the API is designed, requiring all shaders
+ * to be specialized with an entry point, makes supporting this quite
+ * undefined.
+ */
+ if (prog->_LinkedShaders[shader_type]) {
+ prog->data->LinkStatus = linking_failure;
+ return;
+ }
+
+ assert(shader->spirv_data);
+
+ struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader);
+ linked->Stage = shader_type;
+
+ /* Create program and attach it to the linked shader */
+ struct gl_program *gl_prog =
+ ctx->Driver.NewProgram(ctx,
+ _mesa_shader_stage_to_program(shader_type),
+ prog->Name, false);
+ if (!gl_prog) {
+ prog->data->LinkStatus = linking_failure;
+ _mesa_delete_linked_shader(ctx, linked);
+ return;
+ }
+
+ _mesa_reference_shader_program_data(ctx,
+ &gl_prog->sh.data,
+ prog->data);
+
+ /* Don't use _mesa_reference_program() just take ownership */
+ linked->Program = gl_prog;
+
+ /* Reference the SPIR-V data from shader to the linked shader */
+ _mesa_shader_spirv_data_reference(&linked->spirv_data,
+ shader->spirv_data);
+
+ prog->_LinkedShaders[shader_type] = linked;
+ prog->data->linked_stages |= 1 << shader_type;
+ }
}

void GLAPIENTRY

[Timothy Arceri]

Post by Eduardo Lima Mitev
v2: Bail out if we see more that one shader for the same stage, and add
a corresponding comment. (Timothy Arceri)
---
src/mesa/main/glspirv.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 58 insertions(+), 3 deletions(-)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e533853f7fa..0934ceccbb3 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -29,6 +29,8 @@
#include "compiler/nir/nir.h"
#include "compiler/spirv/nir_spirv.h"
+#include "program/program.h"
+
#include "util/u_atomic.h"
void
@@ -104,14 +106,67 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
}
+/**
+ * This is the equivalent to compiler/glsl/linker.cpp::link_shaders()
+ * but for SPIR-V programs.
+ *
+ * This method just creates the gl_linked_shader structs with a reference to
+ * the SPIR-V data collected during previous steps.
+ *
+ * The real linking happens later in the driver-specifc call LinkShader().
+ * This is so backends can implement different linking strategies for
+ * SPIR-V programs.
+ */
void
_mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
{
- * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
- */
prog->data->LinkStatus = linking_success;
prog->data->Validated = false;
+
+ for (unsigned i = 0; i < prog->NumShaders; i++) {
+ struct gl_shader *shader = prog->Shaders[i];
+ gl_shader_stage shader_type = shader->Stage;
+
+ /* We only support one shader per stage. The gl_spirv spec doesn't seem
+ * to prevent this, but the way the API is designed, requiring all shaders
+ * to be specialized with an entry point, makes supporting this quite
+ * undefined.
+ */
+ if (prog->_LinkedShaders[shader_type]) {

We should probably report an error here too right? Otherwise we would
fail to link and the user would have no idea why.

Post by Eduardo Lima Mitev
+ prog->data->LinkStatus = linking_failure;
+ return;
+ }
+
+ assert(shader->spirv_data);
+
+ struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader);
+ linked->Stage = shader_type;
+
+ /* Create program and attach it to the linked shader */
+ struct gl_program *gl_prog =
+ ctx->Driver.NewProgram(ctx,
+ _mesa_shader_stage_to_program(shader_type),
+ prog->Name, false);
+ if (!gl_prog) {
+ prog->data->LinkStatus = linking_failure;
+ _mesa_delete_linked_shader(ctx, linked);
+ return;
+ }
+
+ _mesa_reference_shader_program_data(ctx,
+ &gl_prog->sh.data,
+ prog->data);
+
+ /* Don't use _mesa_reference_program() just take ownership */
+ linked->Program = gl_prog;
+
+ /* Reference the SPIR-V data from shader to the linked shader */
+ _mesa_shader_spirv_data_reference(&linked->spirv_data,
+ shader->spirv_data);
+
+ prog->_LinkedShaders[shader_type] = linked;
+ prog->data->linked_stages |= 1 << shader_type;
+ }
}
void GLAPIENTRY

[Eduardo Lima Mitev]

v2: Bail out if we see more that one shader for the same stage, and add
a corresponding comment. (Timothy Arceri)

v3: Adds also a linker error log to the condition above. (Timothy Arceri)
---
src/mesa/main/glspirv.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 61 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index f90b4f054a6..ad5b467169e 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -29,6 +29,8 @@
#include "compiler/nir/nir.h"
#include "compiler/spirv/nir_spirv.h"

+#include "program/program.h"
+
#include "util/u_atomic.h"

void
@@ -104,14 +106,70 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
}

+/**
+ * This is the equivalent to compiler/glsl/linker.cpp::link_shaders()
+ * but for SPIR-V programs.
+ *
+ * This method just creates the gl_linked_shader structs with a reference to
+ * the SPIR-V data collected during previous steps.
+ *
+ * The real linking happens later in the driver-specifc call LinkShader().
+ * This is so backends can implement different linking strategies for
+ * SPIR-V programs.
+ */
void
_mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
{
- /* @TODO: This is a placeholder for the equivalent of
- * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
- */
prog->data->LinkStatus = linking_success;
prog->data->Validated = false;
+
+ for (unsigned i = 0; i < prog->NumShaders; i++) {
+ struct gl_shader *shader = prog->Shaders[i];
+ gl_shader_stage shader_type = shader->Stage;
+
+ /* We only support one shader per stage. The gl_spirv spec doesn't seem
+ * to prevent this, but the way the API is designed, requiring all shaders
+ * to be specialized with an entry point, makes supporting this quite
+ * undefined.
+ */
+ if (prog->_LinkedShaders[shader_type]) {
+ ralloc_strcat(&prog->data->InfoLog,
+ "\nError trying to link more than one SPIR-V shader "
+ "per stage.\n");
+ prog->data->LinkStatus = linking_failure;
+ return;
+ }
+
+ assert(shader->spirv_data);
+
+ struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader);
+ linked->Stage = shader_type;
+
+ /* Create program and attach it to the linked shader */
+ struct gl_program *gl_prog =
+ ctx->Driver.NewProgram(ctx,
+ _mesa_shader_stage_to_program(shader_type),
+ prog->Name, false);
+ if (!gl_prog) {
+ prog->data->LinkStatus = linking_failure;
+ _mesa_delete_linked_shader(ctx, linked);
+ return;
+ }
+
+ _mesa_reference_shader_program_data(ctx,
+ &gl_prog->sh.data,
+ prog->data);
+
+ /* Don't use _mesa_reference_program() just take ownership */
+ linked->Program = gl_prog;
+
+ /* Reference the SPIR-V data from shader to the linked shader */
+ _mesa_shader_spirv_data_reference(&linked->spirv_data,
+ shader->spirv_data);
+
+ prog->_LinkedShaders[shader_type] = linked;
+ prog->data->linked_stages |= 1 << shader_type;
+ }
}

void GLAPIENTRY

[Eduardo Lima Mitev]

This is the main fork of the shader compilation code-path, where a NIR
shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
depending on its nature..

v2: Use 'spirv_data' member from gl_linked_shader to know which method
to call. (Timothy Arceri)
---
src/mesa/drivers/dri/i965/brw_program.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
index 755d4973cc0..4043253a653 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -31,6 +31,7 @@

#include <pthread.h>
#include "main/imports.h"
+#include "main/glspirv.h"
#include "program/prog_parameter.h"
#include "program/prog_print.h"
#include "program/prog_to_nir.h"
@@ -73,9 +74,14 @@ brw_create_nir(struct brw_context *brw,
ctx->Const.ShaderCompilerOptions[stage].NirOptions;
nir_shader *nir;

- /* First, lower the GLSL IR or Mesa IR to NIR */
+ /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
if (shader_prog) {
- nir = glsl_to_nir(shader_prog, stage, options);
+ if (shader_prog->_LinkedShaders[stage]->spirv_data)
+ nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
+ else
+ nir = glsl_to_nir(shader_prog, stage, options);
+ assert (nir);
+
nir_remove_dead_variables(nir, nir_var_shader_in | nir_var_shader_out);
nir_lower_returns(nir);
nir_validate_shader(nir);

[Timothy Arceri]

Post by Eduardo Lima Mitev
This is the main fork of the shader compilation code-path, where a NIR
shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
depending on its nature..
v2: Use 'spirv_data' member from gl_linked_shader to know which method
to call. (Timothy Arceri)
---
src/mesa/drivers/dri/i965/brw_program.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
index 755d4973cc0..4043253a653 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -31,6 +31,7 @@
#include <pthread.h>
#include "main/imports.h"
+#include "main/glspirv.h"
#include "program/prog_parameter.h"
#include "program/prog_print.h"
#include "program/prog_to_nir.h"
@@ -73,9 +74,14 @@ brw_create_nir(struct brw_context *brw,
ctx->Const.ShaderCompilerOptions[stage].NirOptions;
nir_shader *nir;
- /* First, lower the GLSL IR or Mesa IR to NIR */
+ /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
if (shader_prog) {
- nir = glsl_to_nir(shader_prog, stage, options);
+ if (shader_prog->_LinkedShaders[stage]->spirv_data)
+ nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
+ else
+ nir = glsl_to_nir(shader_prog, stage, options);
+ assert (nir);
+
nir_remove_dead_variables(nir, nir_var_shader_in | nir_var_shader_out);
nir_lower_returns(nir);
nir_validate_shader(nir);

[Eduardo Lima Mitev]

Thanks, Timothy.

Any chance to review the remaining patches in the series?
It would be nice to land this batch soon to focus on the actual linker
stuff.

cheers,
Eduardo

Post by Eduardo Lima Mitev
This is the main fork of the shader compilation code-path, where a NIR
shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
depending on its nature..
v2: Use 'spirv_data' member from gl_linked_shader to know which method
    to call. (Timothy Arceri)
---
  src/mesa/drivers/dri/i965/brw_program.c | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_program.c
b/src/mesa/drivers/dri/i965/brw_program.c
index 755d4973cc0..4043253a653 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -31,6 +31,7 @@
    #include <pthread.h>
  #include "main/imports.h"
+#include "main/glspirv.h"
  #include "program/prog_parameter.h"
  #include "program/prog_print.h"
  #include "program/prog_to_nir.h"
@@ -73,9 +74,14 @@ brw_create_nir(struct brw_context *brw,
        ctx->Const.ShaderCompilerOptions[stage].NirOptions;
     nir_shader *nir;
  -   /* First, lower the GLSL IR or Mesa IR to NIR */
+   /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
     if (shader_prog) {
-      nir = glsl_to_nir(shader_prog, stage, options);
+      if (shader_prog->_LinkedShaders[stage]->spirv_data)
+         nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
+      else
+         nir = glsl_to_nir(shader_prog, stage, options);
+      assert (nir);
+
        nir_remove_dead_variables(nir, nir_var_shader_in |
nir_var_shader_out);
        nir_lower_returns(nir);
        nir_validate_shader(nir);

[Timothy Arceri]

Post by Eduardo Lima Mitev
v2: Bail out if we see more that one shader for the same stage, and add
a corresponding comment. (Timothy Arceri)
v3: Adds also a linker error log to the condition above. (Timothy Arceri)
---
src/mesa/main/glspirv.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 61 insertions(+), 3 deletions(-)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index f90b4f054a6..ad5b467169e 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -29,6 +29,8 @@
#include "compiler/nir/nir.h"
#include "compiler/spirv/nir_spirv.h"
+#include "program/program.h"
+
#include "util/u_atomic.h"
void
@@ -104,14 +106,70 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
}
+/**
+ * This is the equivalent to compiler/glsl/linker.cpp::link_shaders()
+ * but for SPIR-V programs.
+ *
+ * This method just creates the gl_linked_shader structs with a reference to
+ * the SPIR-V data collected during previous steps.
+ *
+ * The real linking happens later in the driver-specifc call LinkShader().
+ * This is so backends can implement different linking strategies for
+ * SPIR-V programs.
+ */
void
_mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
{
- * compiler/glsl/linker.cpp::link_shaders() but for SPIR-V.
- */
prog->data->LinkStatus = linking_success;
prog->data->Validated = false;
+
+ for (unsigned i = 0; i < prog->NumShaders; i++) {
+ struct gl_shader *shader = prog->Shaders[i];
+ gl_shader_stage shader_type = shader->Stage;
+
+ /* We only support one shader per stage. The gl_spirv spec doesn't seem
+ * to prevent this, but the way the API is designed, requiring all shaders
+ * to be specialized with an entry point, makes supporting this quite
+ * undefined.
+ */
+ if (prog->_LinkedShaders[shader_type]) {

Have you guys reported this as a spec bug? Can you add something like:

/* TODO: Turn this into a proper error once the spec bug
---link/refeence to bug here--- is resolved
*/

With that this is:

Reviewed-by: Timothy Arceri <***@itsqueeze.com>

Thanks!

Post by Eduardo Lima Mitev
+ ralloc_strcat(&prog->data->InfoLog,
+ "\nError trying to link more than one SPIR-V shader "
+ "per stage.\n");
+ prog->data->LinkStatus = linking_failure;
+ return;
+ }
+
+ assert(shader->spirv_data);
+
+ struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader);
+ linked->Stage = shader_type;
+
+ /* Create program and attach it to the linked shader */
+ struct gl_program *gl_prog =
+ ctx->Driver.NewProgram(ctx,
+ _mesa_shader_stage_to_program(shader_type),
+ prog->Name, false);
+ if (!gl_prog) {
+ prog->data->LinkStatus = linking_failure;
+ _mesa_delete_linked_shader(ctx, linked);
+ return;
+ }
+
+ _mesa_reference_shader_program_data(ctx,
+ &gl_prog->sh.data,
+ prog->data);
+
+ /* Don't use _mesa_reference_program() just take ownership */
+ linked->Program = gl_prog;
+
+ /* Reference the SPIR-V data from shader to the linked shader */
+ _mesa_shader_spirv_data_reference(&linked->spirv_data,
+ shader->spirv_data);
+
+ prog->_LinkedShaders[shader_type] = linked;
+ prog->data->linked_stages |= 1 << shader_type;
+ }
}
void GLAPIENTRY

[Eduardo Lima Mitev]

This is basically a wrapper around spirv_to_nir() that includes
arguments setup and post-conversion validation.

v2: Rebase update (SpirVCapabilities not a pointer anymore)
---
src/mesa/main/glspirv.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 7 ++++++
2 files changed, 67 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e5dc8b26ea9..2a20e4b5cc7 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -159,6 +159,66 @@ _mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
}
}

+nir_shader *
+_mesa_spirv_to_nir(struct gl_context *ctx,
+ const struct gl_shader_program *prog,
+ gl_shader_stage stage,
+ const nir_shader_compiler_options *options)
+{
+ nir_shader *nir = NULL;
+
+ struct gl_linked_shader *linked_shader = prog->_LinkedShaders[stage];
+ assert (linked_shader);
+
+ struct gl_shader_spirv_data *spirv_data = linked_shader->spirv_data;
+ assert(spirv_data);
+
+ struct gl_spirv_module *spirv_module = spirv_data->SpirVModule;
+ assert (spirv_module != NULL);
+
+ const char *entry_point_name = spirv_data->SpirVEntryPoint;
+ assert(entry_point_name);
+
+ struct nir_spirv_specialization *spec_entries = NULL;
+ spec_entries = calloc(sizeof(*spec_entries),
+ spirv_data->NumSpecializationConstants);
+
+ for (unsigned i = 0; i < spirv_data->NumSpecializationConstants; ++i) {
+ spec_entries[i].id = spirv_data->SpecializationConstantsIndex[i];
+ spec_entries[i].data32 = spirv_data->SpecializationConstantsValue[i];
+ spec_entries[i].defined_on_module = false;
+ }
+
+ nir_function *entry_point =
+ spirv_to_nir((const uint32_t *) &spirv_module->Binary[0],
+ spirv_module->Length / 4,
+ spec_entries, spirv_data->NumSpecializationConstants,
+ stage, entry_point_name,
+ &ctx->Const.SpirVCapabilities,
+ options);
+ free(spec_entries);
+
+ assert (entry_point);
+ nir = entry_point->shader;
+ assert(nir->info.stage == stage);
+
+ nir->options = options;
+
+ nir->info.name =
+ ralloc_asprintf(nir, "SPIRV:%s:%d",
+ _mesa_shader_stage_to_abbrev(nir->info.stage),
+ prog->Name);
+ nir_validate_shader(nir);
+
+ if (false) {
+ /* @FIXME: Only for debugging purposes */
+ nir_print_shader(nir, stdout);
+ fflush(stdout);
+ }
+
+ return nir;
+}
+
void GLAPIENTRY
_mesa_SpecializeShaderARB(GLuint shader,
const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index 0f03b75c111..81626ce75b5 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -24,6 +24,7 @@
#ifndef GLSPIRV_H
#define GLSPIRV_H

+#include "compiler/nir/nir.h"
#include "mtypes.h"

#ifdef __cplusplus
@@ -80,6 +81,12 @@ void
_mesa_spirv_link_shaders(struct gl_context *ctx,
struct gl_shader_program *prog);

+nir_shader *
+_mesa_spirv_to_nir(struct gl_context *ctx,
+ const struct gl_shader_program *prog,
+ gl_shader_stage stage,
+ const nir_shader_compiler_options *options);
+
/**
* \name API functions
*/

[Timothy Arceri]

Post by Eduardo Lima Mitev
This is basically a wrapper around spirv_to_nir() that includes
arguments setup and post-conversion validation.
v2: Rebase update (SpirVCapabilities not a pointer anymore)
---
src/mesa/main/glspirv.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 7 ++++++
2 files changed, 67 insertions(+)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e5dc8b26ea9..2a20e4b5cc7 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -159,6 +159,66 @@ _mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
}
}
+nir_shader *
+_mesa_spirv_to_nir(struct gl_context *ctx,
+ const struct gl_shader_program *prog,
+ gl_shader_stage stage,
+ const nir_shader_compiler_options *options)
+{
+ nir_shader *nir = NULL;
+
+ struct gl_linked_shader *linked_shader = prog->_LinkedShaders[stage];
+ assert (linked_shader);
+
+ struct gl_shader_spirv_data *spirv_data = linked_shader->spirv_data;
+ assert(spirv_data);
+
+ struct gl_spirv_module *spirv_module = spirv_data->SpirVModule;
+ assert (spirv_module != NULL);
+
+ const char *entry_point_name = spirv_data->SpirVEntryPoint;
+ assert(entry_point_name);
+
+ struct nir_spirv_specialization *spec_entries = NULL;
+ spec_entries = calloc(sizeof(*spec_entries),
+ spirv_data->NumSpecializationConstants);

Can we just make this:

struct nir_spirv_specialization *spec_entries =
calloc(sizeof(*spec_entries),
spirv_data->NumSpecializationConstants);

Post by Eduardo Lima Mitev
+
+ for (unsigned i = 0; i < spirv_data->NumSpecializationConstants; ++i) {
+ spec_entries[i].id = spirv_data->SpecializationConstantsIndex[i];
+ spec_entries[i].data32 = spirv_data->SpecializationConstantsValue[i];
+ spec_entries[i].defined_on_module = false;
+ }
+
+ nir_function *entry_point =
+ spirv_to_nir((const uint32_t *) &spirv_module->Binary[0],
+ spirv_module->Length / 4,
+ spec_entries, spirv_data->NumSpecializationConstants,
+ stage, entry_point_name,
+ &ctx->Const.SpirVCapabilities,
+ options);
+ free(spec_entries);
+
+ assert (entry_point);
+ nir = entry_point->shader;
+ assert(nir->info.stage == stage);
+
+ nir->options = options;
+
+ nir->info.name =
+ ralloc_asprintf(nir, "SPIRV:%s:%d",
+ _mesa_shader_stage_to_abbrev(nir->info.stage),
+ prog->Name);
+ nir_validate_shader(nir);
+
+ if (false) {
+ nir_print_shader(nir, stdout);
+ fflush(stdout);
+ }

I'd rather we not commit this debug code, if you want this for
development please carry a patch around in your dev branch.

Post by Eduardo Lima Mitev
+
+ return nir;
+}
+
void GLAPIENTRY
_mesa_SpecializeShaderARB(GLuint shader,
const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index 0f03b75c111..81626ce75b5 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -24,6 +24,7 @@
#ifndef GLSPIRV_H
#define GLSPIRV_H
+#include "compiler/nir/nir.h"
#include "mtypes.h"
#ifdef __cplusplus
@@ -80,6 +81,12 @@ void
_mesa_spirv_link_shaders(struct gl_context *ctx,
struct gl_shader_program *prog);
+nir_shader *
+_mesa_spirv_to_nir(struct gl_context *ctx,
+ const struct gl_shader_program *prog,
+ gl_shader_stage stage,
+ const nir_shader_compiler_options *options);
+
/**
* \name API functions
*/

[Eduardo Lima Mitev]

Post by Eduardo Lima Mitev
This is basically a wrapper around spirv_to_nir() that includes
arguments setup and post-conversion validation.
v2: Rebase update (SpirVCapabilities not a pointer anymore)
---
  src/mesa/main/glspirv.c | 60
+++++++++++++++++++++++++++++++++++++++++++++++++
  src/mesa/main/glspirv.h |  7 ++++++
  2 files changed, 67 insertions(+)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e5dc8b26ea9..2a20e4b5cc7 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -159,6 +159,66 @@ _mesa_spirv_link_shaders(struct gl_context *ctx,
struct gl_shader_program *prog)
     }
  }
  +nir_shader *
+_mesa_spirv_to_nir(struct gl_context *ctx,
+                   const struct gl_shader_program *prog,
+                   gl_shader_stage stage,
+                   const nir_shader_compiler_options *options)
+{
+   nir_shader *nir = NULL;
+
+   struct gl_linked_shader *linked_shader =
prog->_LinkedShaders[stage];
+   assert (linked_shader);
+
+   struct gl_shader_spirv_data *spirv_data = linked_shader->spirv_data;
+   assert(spirv_data);
+
+   struct gl_spirv_module *spirv_module = spirv_data->SpirVModule;
+   assert (spirv_module != NULL);
+
+   const char *entry_point_name = spirv_data->SpirVEntryPoint;
+   assert(entry_point_name);
+
+   struct nir_spirv_specialization *spec_entries = NULL;
+   spec_entries = calloc(sizeof(*spec_entries),
+                         spirv_data->NumSpecializationConstants);

   struct nir_spirv_specialization *spec_entries =
      calloc(sizeof(*spec_entries),
             spirv_data->NumSpecializationConstants);

Sure, will fix it locally.

Post by Eduardo Lima Mitev
+
+   for (unsigned i = 0; i < spirv_data->NumSpecializationConstants;
++i) {
+      spec_entries[i].id = spirv_data->SpecializationConstantsIndex[i];
+      spec_entries[i].data32 =
spirv_data->SpecializationConstantsValue[i];
+      spec_entries[i].defined_on_module = false;
+   }
+
+   nir_function *entry_point =
+      spirv_to_nir((const uint32_t *) &spirv_module->Binary[0],
+                   spirv_module->Length / 4,
+                   spec_entries,
spirv_data->NumSpecializationConstants,
+                   stage, entry_point_name,
+                   &ctx->Const.SpirVCapabilities,
+                   options);
+   free(spec_entries);
+
+   assert (entry_point);
+   nir = entry_point->shader;
+   assert(nir->info.stage == stage);
+
+   nir->options = options;
+
+   nir->info.name =
+      ralloc_asprintf(nir, "SPIRV:%s:%d",
+                      _mesa_shader_stage_to_abbrev(nir->info.stage),
+                      prog->Name);
+   nir_validate_shader(nir);
+
+   if (false) {
+      nir_print_shader(nir, stdout);
+      fflush(stdout);
+   }

I'd rather we not commit this debug code, if you want this for
development please carry a patch around in your dev branch.

Agree. Will remove it locally.
Thanks!

Post by Eduardo Lima Mitev
+
+   return nir;
+}
+
  void GLAPIENTRY
  _mesa_SpecializeShaderARB(GLuint shader,
                            const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index 0f03b75c111..81626ce75b5 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -24,6 +24,7 @@
  #ifndef GLSPIRV_H
  #define GLSPIRV_H
  +#include "compiler/nir/nir.h"
  #include "mtypes.h"
    #ifdef __cplusplus
@@ -80,6 +81,12 @@ void
  _mesa_spirv_link_shaders(struct gl_context *ctx,
                           struct gl_shader_program *prog);
  +nir_shader *
+_mesa_spirv_to_nir(struct gl_context *ctx,
+                   const struct gl_shader_program *prog,
+                   gl_shader_stage stage,
+                   const nir_shader_compiler_options *options);
+
  /**
   * \name API functions
   */

[Eduardo Lima Mitev]

This is the main fork of the shader compilation code-path, where a NIR
shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
depending on its nature..

v2: Use 'spirv_data' member from gl_linked_shader to know which method
to call. (Timothy Arceri)
---
src/mesa/drivers/dri/i965/brw_program.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
index 755d4973cc0..596118f2fe5 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -31,6 +31,7 @@

#include <pthread.h>
#include "main/imports.h"
+#include "main/glspirv.h"
#include "program/prog_parameter.h"
#include "program/prog_print.h"
#include "program/prog_to_nir.h"
@@ -73,9 +74,18 @@ brw_create_nir(struct brw_context *brw,
ctx->Const.ShaderCompilerOptions[stage].NirOptions;
nir_shader *nir;

- /* First, lower the GLSL IR or Mesa IR to NIR */
+ /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
if (shader_prog) {
- nir = glsl_to_nir(shader_prog, stage, options);
+ bool is_spirv_shader =
+ (shader_prog->_LinkedShaders[stage]->spirv_data != NULL);
+
+ if (!is_spirv_shader) {
+ nir = glsl_to_nir(shader_prog, stage, options);
+ } else {
+ nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
+ }
+ assert (nir);
+
nir_remove_dead_variables(nir, nir_var_shader_in | nir_var_shader_out);
nir_lower_returns(nir);
nir_validate_shader(nir);

[Timothy Arceri]

Post by Eduardo Lima Mitev
This is the main fork of the shader compilation code-path, where a NIR
shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
depending on its nature..
v2: Use 'spirv_data' member from gl_linked_shader to know which method
to call. (Timothy Arceri)
---
src/mesa/drivers/dri/i965/brw_program.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
index 755d4973cc0..596118f2fe5 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -31,6 +31,7 @@
#include <pthread.h>
#include "main/imports.h"
+#include "main/glspirv.h"
#include "program/prog_parameter.h"
#include "program/prog_print.h"
#include "program/prog_to_nir.h"
@@ -73,9 +74,18 @@ brw_create_nir(struct brw_context *brw,
ctx->Const.ShaderCompilerOptions[stage].NirOptions;
nir_shader *nir;
- /* First, lower the GLSL IR or Mesa IR to NIR */
+ /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
if (shader_prog) {
- nir = glsl_to_nir(shader_prog, stage, options);
+ bool is_spirv_shader =
+ (shader_prog->_LinkedShaders[stage]->spirv_data != NULL);
+
+ if (!is_spirv_shader) {
+ nir = glsl_to_nir(shader_prog, stage, options);
+ } else {
+ nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
+ }
+ assert (nir);

Rather than messing around with bools, null checks and !'s I'd just
change this to:

/* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
if (shader_prog->_LinkedShaders[stage]->spirv_data) {
nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
} else {
nir = glsl_to_nir(shader_prog, stage, options);
}
assert (nir);

Post by Eduardo Lima Mitev
+
nir_remove_dead_variables(nir, nir_var_shader_in | nir_var_shader_out);
nir_lower_returns(nir);
nir_validate_shader(nir);

[Eduardo Lima Mitev]

Post by Timothy Arceri

Post by Eduardo Lima Mitev
This is the main fork of the shader compilation code-path, where a NIR
shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
depending on its nature..
v2: Use 'spirv_data' member from gl_linked_shader to know which method
    to call. (Timothy Arceri)
---
  src/mesa/drivers/dri/i965/brw_program.c | 14 ++++++++++++--
  1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_program.c
b/src/mesa/drivers/dri/i965/brw_program.c
index 755d4973cc0..596118f2fe5 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -31,6 +31,7 @@
    #include <pthread.h>
  #include "main/imports.h"
+#include "main/glspirv.h"
  #include "program/prog_parameter.h"
  #include "program/prog_print.h"
  #include "program/prog_to_nir.h"
@@ -73,9 +74,18 @@ brw_create_nir(struct brw_context *brw,
        ctx->Const.ShaderCompilerOptions[stage].NirOptions;
     nir_shader *nir;
  -   /* First, lower the GLSL IR or Mesa IR to NIR */
+   /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
     if (shader_prog) {
-      nir = glsl_to_nir(shader_prog, stage, options);
+      bool is_spirv_shader =
+         (shader_prog->_LinkedShaders[stage]->spirv_data != NULL);
+
+      if (!is_spirv_shader) {
+         nir = glsl_to_nir(shader_prog, stage, options);
+      } else {
+         nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
+      }
+      assert (nir);

Rather than messing around with bools, null checks and !'s I'd just
      /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
      if (shader_prog->_LinkedShaders[stage]->spirv_data) {
         nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
      } else {
         nir = glsl_to_nir(shader_prog, stage, options);
      }
      assert (nir);

My intention was to make it clearer that we are using a pointer nullness
to decide code-path.
I don't care much so I will use your inlined version above, maybe just
adding a comment.

Thanks,

Eduardo

Post by Timothy Arceri

Post by Eduardo Lima Mitev
+
        nir_remove_dead_variables(nir, nir_var_shader_in |
nir_var_shader_out);
        nir_lower_returns(nir);
        nir_validate_shader(nir);

[Timothy Arceri]

Post by Eduardo Lima Mitev

Post by Timothy Arceri

Post by Eduardo Lima Mitev
This is the main fork of the shader compilation code-path, where a NIR
shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
depending on its nature..
v2: Use 'spirv_data' member from gl_linked_shader to know which method
    to call. (Timothy Arceri)
---
  src/mesa/drivers/dri/i965/brw_program.c | 14 ++++++++++++--
  1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_program.c
b/src/mesa/drivers/dri/i965/brw_program.c
index 755d4973cc0..596118f2fe5 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -31,6 +31,7 @@
    #include <pthread.h>
  #include "main/imports.h"
+#include "main/glspirv.h"
  #include "program/prog_parameter.h"
  #include "program/prog_print.h"
  #include "program/prog_to_nir.h"
@@ -73,9 +74,18 @@ brw_create_nir(struct brw_context *brw,
        ctx->Const.ShaderCompilerOptions[stage].NirOptions;
     nir_shader *nir;
  -   /* First, lower the GLSL IR or Mesa IR to NIR */
+   /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
     if (shader_prog) {
-      nir = glsl_to_nir(shader_prog, stage, options);
+      bool is_spirv_shader =
+         (shader_prog->_LinkedShaders[stage]->spirv_data != NULL);
+
+      if (!is_spirv_shader) {
+         nir = glsl_to_nir(shader_prog, stage, options);
+      } else {
+         nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
+      }
+      assert (nir);

Rather than messing around with bools, null checks and !'s I'd just
      /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
      if (shader_prog->_LinkedShaders[stage]->spirv_data) {
         nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
      } else {
         nir = glsl_to_nir(shader_prog, stage, options);
      }
      assert (nir);

My intention was to make it clearer that we are using a pointer nullness
to decide code-path.
I don't care much so I will use your inlined version above, maybe just
adding a comment.

I don't think you even need a comment. The code speaks for itself :)

Post by Eduardo Lima Mitev
Thanks,
Eduardo

Post by Timothy Arceri

Post by Eduardo Lima Mitev
+
        nir_remove_dead_variables(nir, nir_var_shader_in |
nir_var_shader_out);
        nir_lower_returns(nir);
        nir_validate_shader(nir);

[Eduardo Lima Mitev]

v2: Use 'spirv_data' from gl_linked_shader instead, to check if shader
is SPIR-V. (Timothy Arceri)
---
src/mesa/drivers/dri/i965/brw_link.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
index d18521e792d..6bf4c413db4 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -236,7 +236,8 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
struct gl_program *prog = shader->Program;
prog->Parameters = _mesa_new_parameter_list();

- process_glsl_ir(brw, shProg, shader);
+ if (!shader->spirv_data)
+ process_glsl_ir(brw, shProg, shader);

_mesa_copy_linked_program_data(shProg, shader);

[Timothy Arceri]

Post by Eduardo Lima Mitev
v2: Use 'spirv_data' from gl_linked_shader instead, to check if shader
is SPIR-V. (Timothy Arceri)
---
src/mesa/drivers/dri/i965/brw_link.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
index d18521e792d..6bf4c413db4 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -236,7 +236,8 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
struct gl_program *prog = shader->Program;
prog->Parameters = _mesa_new_parameter_list();
- process_glsl_ir(brw, shProg, shader);
+ if (!shader->spirv_data)
+ process_glsl_ir(brw, shProg, shader);
_mesa_copy_linked_program_data(shProg, shader);

[Timothy Arceri]

Post by Eduardo Lima Mitev
Hello,
This is the second version of the series providing initial support for ARB_gl_spirv and ARB_spirv_extensions in Mesa and i965.
First version of the series can be found at <https://lists.freedesktop.org/archives/mesa-dev/2017-November/177004.html>.
In this series we hope we have addressed all issues detected during the initial review. Thank you all who participated!
* A dedicated 'spirv' flag was removed from gl_shader struct. Now we use the nulness of 'spirv_data' member for the same purpose.
* The per-program 'spirv' flag was moved out of this series, but will likely be re-introduced in the next delivery, because it will become necessary.
* We enforce one SPIR-V shader per stage, and fail linking if this condition is not met.

Sorry can you point me to the patch that contains this I couldn't find
it when skimming over the series. Thanks.

Post by Eduardo Lima Mitev
* 'SpirVCapabilities' struct of GL context constants is no longer a pointer but a static struct.
As usual, a tree of this series can be found at <https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v2>.
A tree of the larger WIP branch from which this series is taken: <https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>.
Thanks in advance for the reviews!
cheers,
Eduardo
spirv_extensions: rename nir_spirv_supported_extensions
mesa: move nir_spirv_supported_capabilities definition
i965: initialize SPIR-V capabilities
spirv_extensions: add GL_ARB_spirv_extensions boilerplate
spirv_extensions: add list of extensions and to_string method
spirv_extensions: define spirv_extensions_supported
spirv_extensions: add spirv_supported_extensions on gl_constants
spirv_extensions: i965: initialize SPIR-V extensions
nir/spirv: add gl_spirv_validation method
mesa/glspirv: Add struct gl_shader_spirv_data
mesa/glspirv: Add a _mesa_spirv_link_shaders() placeholder
mesa/program: Link SPIR-V shaders using the SPIR-V code-path
mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader
mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program
mesa/glspirv: Add a _mesa_spirv_to_nir() function
i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders
i965: Don't call process_glsl_ir() for SPIR-V shaders
mesa: Add boilerplate for the GL 4.6 alias of glSpecializeShaderARB
mesa: add GL_ARB_gl_spirv boilerplate
mesa/glspirv: Add struct gl_spirv_module
mesa: implement SPIR-V loading in glShaderBinary
mesa/shaderapi: add a getter for GL_SPIR_V_BINARY_ARB
mesa: refuse to compile SPIR-V shaders or link mixed shaders
mesa: add gl_constants::SpirVCapabilities
mesa: Implement glSpecializeShaderARB
src/amd/vulkan/radv_shader.c | 4 +-
src/compiler/Makefile.sources | 2 +
src/compiler/spirv/nir_spirv.h | 21 +-
src/compiler/spirv/spirv_extensions.c | 77 +++++++
src/compiler/spirv/spirv_extensions.h | 63 ++++++
src/compiler/spirv/spirv_to_nir.c | 160 +++++++++++++-
src/compiler/spirv/vtn_private.h | 2 +-
src/intel/vulkan/anv_pipeline.c | 4 +-
src/mapi/glapi/gen/ARB_gl_spirv.xml | 21 ++
src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 ++
src/mapi/glapi/gen/GL4x.xml | 11 +
src/mapi/glapi/gen/Makefile.am | 2 +
src/mapi/glapi/gen/gl_API.xml | 8 +
src/mapi/glapi/gen/gl_genexec.py | 1 +
src/mapi/glapi/gen/meson.build | 2 +
src/mesa/Makefile.sources | 4 +
src/mesa/drivers/dri/i965/brw_context.c | 26 +++
src/mesa/drivers/dri/i965/brw_link.cpp | 3 +-
src/mesa/drivers/dri/i965/brw_program.c | 14 +-
src/mesa/main/context.c | 2 +
src/mesa/main/extensions_table.h | 2 +
src/mesa/main/get.c | 7 +
src/mesa/main/get_hash_params.py | 3 +
src/mesa/main/getstring.c | 12 +
src/mesa/main/glspirv.c | 331 ++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 108 +++++++++
src/mesa/main/mtypes.h | 31 +++
src/mesa/main/shaderapi.c | 60 ++++-
src/mesa/main/shaderobj.c | 3 +
src/mesa/main/spirv_extensions.c | 60 +++++
src/mesa/main/spirv_extensions.h | 49 ++++
src/mesa/main/tests/dispatch_sanity.cpp | 3 +
src/mesa/meson.build | 4 +
src/mesa/program/ir_to_mesa.cpp | 23 +-
34 files changed, 1098 insertions(+), 38 deletions(-)
create mode 100644 src/compiler/spirv/spirv_extensions.c
create mode 100644 src/compiler/spirv/spirv_extensions.h
create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml
create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
create mode 100644 src/mesa/main/glspirv.c
create mode 100644 src/mesa/main/glspirv.h
create mode 100644 src/mesa/main/spirv_extensions.c
create mode 100644 src/mesa/main/spirv_extensions.h

[Eduardo Lima Mitev]

Post by Timothy Arceri

Post by Eduardo Lima Mitev
Hello,
This is the second version of the series providing initial support
for ARB_gl_spirv and ARB_spirv_extensions in Mesa and i965.
First version of the series can be found at
<https://lists.freedesktop.org/archives/mesa-dev/2017-November/177004.html>.
In this series we hope we have addressed all issues detected during
the initial review. Thank you all who participated!
Taking the nitpicks and minor fixes apart, most important changes
* A dedicated 'spirv' flag was removed from gl_shader struct. Now we
use the nulness of 'spirv_data' member for the same purpose.
* The per-program 'spirv' flag was moved out of this series, but will
likely be re-introduced in the next delivery, because it will become
necessary.
* We enforce one SPIR-V shader per stage, and fail linking if this condition is not met.

Sorry can you point me to the patch that contains this I couldn't find
it when skimming over the series. Thanks.

Ohh, I'm very sorry. The revised patch got lost during rebase.

I have just sent a v3 of
"[PATCH v2 22/25] mesa/glspirv: Create gl_linked_shader objects for a
SPIR-V program" that includes the check in question.

Thanks for catching this!

Eduardo

Post by Timothy Arceri

Post by Eduardo Lima Mitev
* 'SpirVCapabilities' struct of GL context constants is no longer a
pointer but a static struct.
As usual, a tree of this series can be found at
<https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v2>.
<https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>.
Thanks in advance for the reviews!
cheers,
Eduardo
   spirv_extensions: rename nir_spirv_supported_extensions
   mesa: move nir_spirv_supported_capabilities definition
   i965: initialize SPIR-V capabilities
   spirv_extensions: add GL_ARB_spirv_extensions boilerplate
   spirv_extensions: add list of extensions and to_string method
   spirv_extensions: define spirv_extensions_supported
   spirv_extensions: add spirv_supported_extensions on gl_constants
   spirv_extensions: i965: initialize SPIR-V extensions
   nir/spirv: add gl_spirv_validation method
   mesa/glspirv: Add struct gl_shader_spirv_data
   mesa/glspirv: Add a _mesa_spirv_link_shaders() placeholder
   mesa/program: Link SPIR-V shaders using the SPIR-V code-path
   mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader
   mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program
   mesa/glspirv: Add a _mesa_spirv_to_nir() function
   i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders
   i965: Don't call process_glsl_ir() for SPIR-V shaders
   mesa: Add boilerplate for the GL 4.6 alias of glSpecializeShaderARB
   mesa: add GL_ARB_gl_spirv boilerplate
   mesa/glspirv: Add struct gl_spirv_module
   mesa: implement SPIR-V loading in glShaderBinary
   mesa/shaderapi: add a getter for GL_SPIR_V_BINARY_ARB
   mesa: refuse to compile SPIR-V shaders or link mixed shaders
   mesa: add gl_constants::SpirVCapabilities
   mesa: Implement glSpecializeShaderARB
  src/amd/vulkan/radv_shader.c                |   4 +-
  src/compiler/Makefile.sources               |   2 +
  src/compiler/spirv/nir_spirv.h              |  21 +-
  src/compiler/spirv/spirv_extensions.c       |  77 +++++++
  src/compiler/spirv/spirv_extensions.h       |  63 ++++++
  src/compiler/spirv/spirv_to_nir.c           | 160 +++++++++++++-
  src/compiler/spirv/vtn_private.h            |   2 +-
  src/intel/vulkan/anv_pipeline.c             |   4 +-
  src/mapi/glapi/gen/ARB_gl_spirv.xml         |  21 ++
  src/mapi/glapi/gen/ARB_spirv_extensions.xml |  13 ++
  src/mapi/glapi/gen/GL4x.xml                 |  11 +
  src/mapi/glapi/gen/Makefile.am              |   2 +
  src/mapi/glapi/gen/gl_API.xml               |   8 +
  src/mapi/glapi/gen/gl_genexec.py            |   1 +
  src/mapi/glapi/gen/meson.build              |   2 +
  src/mesa/Makefile.sources                   |   4 +
  src/mesa/drivers/dri/i965/brw_context.c     |  26 +++
  src/mesa/drivers/dri/i965/brw_link.cpp      |   3 +-
  src/mesa/drivers/dri/i965/brw_program.c     |  14 +-
  src/mesa/main/context.c                     |   2 +
  src/mesa/main/extensions_table.h            |   2 +
  src/mesa/main/get.c                         |   7 +
  src/mesa/main/get_hash_params.py            |   3 +
  src/mesa/main/getstring.c                   |  12 +
  src/mesa/main/glspirv.c                     | 331
++++++++++++++++++++++++++++
  src/mesa/main/glspirv.h                     | 108 +++++++++
  src/mesa/main/mtypes.h                      |  31 +++
  src/mesa/main/shaderapi.c                   |  60 ++++-
  src/mesa/main/shaderobj.c                   |   3 +
  src/mesa/main/spirv_extensions.c            |  60 +++++
  src/mesa/main/spirv_extensions.h            |  49 ++++
  src/mesa/main/tests/dispatch_sanity.cpp     |   3 +
  src/mesa/meson.build                        |   4 +
  src/mesa/program/ir_to_mesa.cpp             |  23 +-
  34 files changed, 1098 insertions(+), 38 deletions(-)
  create mode 100644 src/compiler/spirv/spirv_extensions.c
  create mode 100644 src/compiler/spirv/spirv_extensions.h
  create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml
  create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
  create mode 100644 src/mesa/main/glspirv.c
  create mode 100644 src/mesa/main/glspirv.h
  create mode 100644 src/mesa/main/spirv_extensions.c
  create mode 100644 src/mesa/main/spirv_extensions.h

[Ian Romanick]

Any chance you could push an updated branch to your github? :)

Post by Eduardo Lima Mitev
Hello,
This is the second version of the series providing initial support for ARB_gl_spirv and ARB_spirv_extensions in Mesa and i965.
First version of the series can be found at <https://lists.freedesktop.org/archives/mesa-dev/2017-November/177004.html>.
In this series we hope we have addressed all issues detected during the initial review. Thank you all who participated!
* A dedicated 'spirv' flag was removed from gl_shader struct. Now we use the nulness of 'spirv_data' member for the same purpose.
* The per-program 'spirv' flag was moved out of this series, but will likely be re-introduced in the next delivery, because it will become necessary.
* We enforce one SPIR-V shader per stage, and fail linking if this condition is not met.
* 'SpirVCapabilities' struct of GL context constants is no longer a pointer but a static struct.
As usual, a tree of this series can be found at <https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v2>.
A tree of the larger WIP branch from which this series is taken: <https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>.
Thanks in advance for the reviews!
cheers,
Eduardo
spirv_extensions: rename nir_spirv_supported_extensions
mesa: move nir_spirv_supported_capabilities definition
i965: initialize SPIR-V capabilities
spirv_extensions: add GL_ARB_spirv_extensions boilerplate
spirv_extensions: add list of extensions and to_string method
spirv_extensions: define spirv_extensions_supported
spirv_extensions: add spirv_supported_extensions on gl_constants
spirv_extensions: i965: initialize SPIR-V extensions
nir/spirv: add gl_spirv_validation method
mesa/glspirv: Add struct gl_shader_spirv_data
mesa/glspirv: Add a _mesa_spirv_link_shaders() placeholder
mesa/program: Link SPIR-V shaders using the SPIR-V code-path
mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader
mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program
mesa/glspirv: Add a _mesa_spirv_to_nir() function
i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders
i965: Don't call process_glsl_ir() for SPIR-V shaders
mesa: Add boilerplate for the GL 4.6 alias of glSpecializeShaderARB
mesa: add GL_ARB_gl_spirv boilerplate
mesa/glspirv: Add struct gl_spirv_module
mesa: implement SPIR-V loading in glShaderBinary
mesa/shaderapi: add a getter for GL_SPIR_V_BINARY_ARB
mesa: refuse to compile SPIR-V shaders or link mixed shaders
mesa: add gl_constants::SpirVCapabilities
mesa: Implement glSpecializeShaderARB
src/amd/vulkan/radv_shader.c | 4 +-
src/compiler/Makefile.sources | 2 +
src/compiler/spirv/nir_spirv.h | 21 +-
src/compiler/spirv/spirv_extensions.c | 77 +++++++
src/compiler/spirv/spirv_extensions.h | 63 ++++++
src/compiler/spirv/spirv_to_nir.c | 160 +++++++++++++-
src/compiler/spirv/vtn_private.h | 2 +-
src/intel/vulkan/anv_pipeline.c | 4 +-
src/mapi/glapi/gen/ARB_gl_spirv.xml | 21 ++
src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 ++
src/mapi/glapi/gen/GL4x.xml | 11 +
src/mapi/glapi/gen/Makefile.am | 2 +
src/mapi/glapi/gen/gl_API.xml | 8 +
src/mapi/glapi/gen/gl_genexec.py | 1 +
src/mapi/glapi/gen/meson.build | 2 +
src/mesa/Makefile.sources | 4 +
src/mesa/drivers/dri/i965/brw_context.c | 26 +++
src/mesa/drivers/dri/i965/brw_link.cpp | 3 +-
src/mesa/drivers/dri/i965/brw_program.c | 14 +-
src/mesa/main/context.c | 2 +
src/mesa/main/extensions_table.h | 2 +
src/mesa/main/get.c | 7 +
src/mesa/main/get_hash_params.py | 3 +
src/mesa/main/getstring.c | 12 +
src/mesa/main/glspirv.c | 331 ++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 108 +++++++++
src/mesa/main/mtypes.h | 31 +++
src/mesa/main/shaderapi.c | 60 ++++-
src/mesa/main/shaderobj.c | 3 +
src/mesa/main/spirv_extensions.c | 60 +++++
src/mesa/main/spirv_extensions.h | 49 ++++
src/mesa/main/tests/dispatch_sanity.cpp | 3 +
src/mesa/meson.build | 4 +
src/mesa/program/ir_to_mesa.cpp | 23 +-
34 files changed, 1098 insertions(+), 38 deletions(-)
create mode 100644 src/compiler/spirv/spirv_extensions.c
create mode 100644 src/compiler/spirv/spirv_extensions.h
create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml
create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
create mode 100644 src/mesa/main/glspirv.c
create mode 100644 src/mesa/main/glspirv.h
create mode 100644 src/mesa/main/spirv_extensions.c
create mode 100644 src/mesa/main/spirv_extensions.h

[Eduardo Lima Mitev]

Post by Ian Romanick
Any chance you could push an updated branch to your github? :)

Yes, sure. I was meant to send the v3 series already yesterday.

I just pushed the latest version of this series
to<https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v2>.

And the full branch to
<https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>.

I'm will send a v3 soon, after I sort out the last review comment.

Eduardo

Post by Ian Romanick

Post by Eduardo Lima Mitev
Hello,
This is the second version of the series providing initial support for ARB_gl_spirv and ARB_spirv_extensions in Mesa and i965.
First version of the series can be found at <https://lists.freedesktop.org/archives/mesa-dev/2017-November/177004.html>.
In this series we hope we have addressed all issues detected during the initial review. Thank you all who participated!
* A dedicated 'spirv' flag was removed from gl_shader struct. Now we use the nulness of 'spirv_data' member for the same purpose.
* The per-program 'spirv' flag was moved out of this series, but will likely be re-introduced in the next delivery, because it will become necessary.
* We enforce one SPIR-V shader per stage, and fail linking if this condition is not met.
* 'SpirVCapabilities' struct of GL context constants is no longer a pointer but a static struct.
As usual, a tree of this series can be found at <https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v2>.
A tree of the larger WIP branch from which this series is taken: <https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>.
Thanks in advance for the reviews!
cheers,
Eduardo
spirv_extensions: rename nir_spirv_supported_extensions
mesa: move nir_spirv_supported_capabilities definition
i965: initialize SPIR-V capabilities
spirv_extensions: add GL_ARB_spirv_extensions boilerplate
spirv_extensions: add list of extensions and to_string method
spirv_extensions: define spirv_extensions_supported
spirv_extensions: add spirv_supported_extensions on gl_constants
spirv_extensions: i965: initialize SPIR-V extensions
nir/spirv: add gl_spirv_validation method
mesa/glspirv: Add struct gl_shader_spirv_data
mesa/glspirv: Add a _mesa_spirv_link_shaders() placeholder
mesa/program: Link SPIR-V shaders using the SPIR-V code-path
mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader
mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program
mesa/glspirv: Add a _mesa_spirv_to_nir() function
i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders
i965: Don't call process_glsl_ir() for SPIR-V shaders
mesa: Add boilerplate for the GL 4.6 alias of glSpecializeShaderARB
mesa: add GL_ARB_gl_spirv boilerplate
mesa/glspirv: Add struct gl_spirv_module
mesa: implement SPIR-V loading in glShaderBinary
mesa/shaderapi: add a getter for GL_SPIR_V_BINARY_ARB
mesa: refuse to compile SPIR-V shaders or link mixed shaders
mesa: add gl_constants::SpirVCapabilities
mesa: Implement glSpecializeShaderARB
src/amd/vulkan/radv_shader.c | 4 +-
src/compiler/Makefile.sources | 2 +
src/compiler/spirv/nir_spirv.h | 21 +-
src/compiler/spirv/spirv_extensions.c | 77 +++++++
src/compiler/spirv/spirv_extensions.h | 63 ++++++
src/compiler/spirv/spirv_to_nir.c | 160 +++++++++++++-
src/compiler/spirv/vtn_private.h | 2 +-
src/intel/vulkan/anv_pipeline.c | 4 +-
src/mapi/glapi/gen/ARB_gl_spirv.xml | 21 ++
src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 ++
src/mapi/glapi/gen/GL4x.xml | 11 +
src/mapi/glapi/gen/Makefile.am | 2 +
src/mapi/glapi/gen/gl_API.xml | 8 +
src/mapi/glapi/gen/gl_genexec.py | 1 +
src/mapi/glapi/gen/meson.build | 2 +
src/mesa/Makefile.sources | 4 +
src/mesa/drivers/dri/i965/brw_context.c | 26 +++
src/mesa/drivers/dri/i965/brw_link.cpp | 3 +-
src/mesa/drivers/dri/i965/brw_program.c | 14 +-
src/mesa/main/context.c | 2 +
src/mesa/main/extensions_table.h | 2 +
src/mesa/main/get.c | 7 +
src/mesa/main/get_hash_params.py | 3 +
src/mesa/main/getstring.c | 12 +
src/mesa/main/glspirv.c | 331 ++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 108 +++++++++
src/mesa/main/mtypes.h | 31 +++
src/mesa/main/shaderapi.c | 60 ++++-
src/mesa/main/shaderobj.c | 3 +
src/mesa/main/spirv_extensions.c | 60 +++++
src/mesa/main/spirv_extensions.h | 49 ++++
src/mesa/main/tests/dispatch_sanity.cpp | 3 +
src/mesa/meson.build | 4 +
src/mesa/program/ir_to_mesa.cpp | 23 +-
34 files changed, 1098 insertions(+), 38 deletions(-)
create mode 100644 src/compiler/spirv/spirv_extensions.c
create mode 100644 src/compiler/spirv/spirv_extensions.h
create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml
create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
create mode 100644 src/mesa/main/glspirv.c
create mode 100644 src/mesa/main/glspirv.h
create mode 100644 src/mesa/main/spirv_extensions.c
create mode 100644 src/mesa/main/spirv_extensions.h

[Eduardo Lima Mitev]

Any chance to wrap up this review?

Thanks!

Eduardo

Post by Eduardo Lima Mitev
Hello,
This is the second version of the series providing initial support for ARB_gl_spirv and ARB_spirv_extensions in Mesa and i965.
First version of the series can be found at <https://lists.freedesktop.org/archives/mesa-dev/2017-November/177004.html>.
In this series we hope we have addressed all issues detected during the initial review. Thank you all who participated!
* A dedicated 'spirv' flag was removed from gl_shader struct. Now we use the nulness of 'spirv_data' member for the same purpose.
* The per-program 'spirv' flag was moved out of this series, but will likely be re-introduced in the next delivery, because it will become necessary.
* We enforce one SPIR-V shader per stage, and fail linking if this condition is not met.
* 'SpirVCapabilities' struct of GL context constants is no longer a pointer but a static struct.
As usual, a tree of this series can be found at <https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v2>.
A tree of the larger WIP branch from which this series is taken: <https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>.
Thanks in advance for the reviews!
cheers,
Eduardo
spirv_extensions: rename nir_spirv_supported_extensions
mesa: move nir_spirv_supported_capabilities definition
i965: initialize SPIR-V capabilities
spirv_extensions: add GL_ARB_spirv_extensions boilerplate
spirv_extensions: add list of extensions and to_string method
spirv_extensions: define spirv_extensions_supported
spirv_extensions: add spirv_supported_extensions on gl_constants
spirv_extensions: i965: initialize SPIR-V extensions
nir/spirv: add gl_spirv_validation method
mesa/glspirv: Add struct gl_shader_spirv_data
mesa/glspirv: Add a _mesa_spirv_link_shaders() placeholder
mesa/program: Link SPIR-V shaders using the SPIR-V code-path
mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader
mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program
mesa/glspirv: Add a _mesa_spirv_to_nir() function
i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders
i965: Don't call process_glsl_ir() for SPIR-V shaders
mesa: Add boilerplate for the GL 4.6 alias of glSpecializeShaderARB
mesa: add GL_ARB_gl_spirv boilerplate
mesa/glspirv: Add struct gl_spirv_module
mesa: implement SPIR-V loading in glShaderBinary
mesa/shaderapi: add a getter for GL_SPIR_V_BINARY_ARB
mesa: refuse to compile SPIR-V shaders or link mixed shaders
mesa: add gl_constants::SpirVCapabilities
mesa: Implement glSpecializeShaderARB
src/amd/vulkan/radv_shader.c | 4 +-
src/compiler/Makefile.sources | 2 +
src/compiler/spirv/nir_spirv.h | 21 +-
src/compiler/spirv/spirv_extensions.c | 77 +++++++
src/compiler/spirv/spirv_extensions.h | 63 ++++++
src/compiler/spirv/spirv_to_nir.c | 160 +++++++++++++-
src/compiler/spirv/vtn_private.h | 2 +-
src/intel/vulkan/anv_pipeline.c | 4 +-
src/mapi/glapi/gen/ARB_gl_spirv.xml | 21 ++
src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 ++
src/mapi/glapi/gen/GL4x.xml | 11 +
src/mapi/glapi/gen/Makefile.am | 2 +
src/mapi/glapi/gen/gl_API.xml | 8 +
src/mapi/glapi/gen/gl_genexec.py | 1 +
src/mapi/glapi/gen/meson.build | 2 +
src/mesa/Makefile.sources | 4 +
src/mesa/drivers/dri/i965/brw_context.c | 26 +++
src/mesa/drivers/dri/i965/brw_link.cpp | 3 +-
src/mesa/drivers/dri/i965/brw_program.c | 14 +-
src/mesa/main/context.c | 2 +
src/mesa/main/extensions_table.h | 2 +
src/mesa/main/get.c | 7 +
src/mesa/main/get_hash_params.py | 3 +
src/mesa/main/getstring.c | 12 +
src/mesa/main/glspirv.c | 331 ++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 108 +++++++++
src/mesa/main/mtypes.h | 31 +++
src/mesa/main/shaderapi.c | 60 ++++-
src/mesa/main/shaderobj.c | 3 +
src/mesa/main/spirv_extensions.c | 60 +++++
src/mesa/main/spirv_extensions.h | 49 ++++
src/mesa/main/tests/dispatch_sanity.cpp | 3 +
src/mesa/meson.build | 4 +
src/mesa/program/ir_to_mesa.cpp | 23 +-
34 files changed, 1098 insertions(+), 38 deletions(-)
create mode 100644 src/compiler/spirv/spirv_extensions.c
create mode 100644 src/compiler/spirv/spirv_extensions.h
create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml
create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
create mode 100644 src/mesa/main/glspirv.c
create mode 100644 src/mesa/main/glspirv.h
create mode 100644 src/mesa/main/spirv_extensions.c
create mode 100644 src/mesa/main/spirv_extensions.h

[Eduardo Lima Mitev]

Oops, sorry, wrong thread.

This is version 2 of the series and there is a version 3 which is the
one that needs review.

Eduardo

Post by Eduardo Lima Mitev
Any chance to wrap up this review?
Thanks!
Eduardo

Post by Eduardo Lima Mitev
Hello,
This is the second version of the series providing initial support for ARB_gl_spirv and ARB_spirv_extensions in Mesa and i965.
First version of the series can be found at <https://lists.freedesktop.org/archives/mesa-dev/2017-November/177004.html>.
In this series we hope we have addressed all issues detected during the initial review. Thank you all who participated!
* A dedicated 'spirv' flag was removed from gl_shader struct. Now we use the nulness of 'spirv_data' member for the same purpose.
* The per-program 'spirv' flag was moved out of this series, but will likely be re-introduced in the next delivery, because it will become necessary.
* We enforce one SPIR-V shader per stage, and fail linking if this condition is not met.
* 'SpirVCapabilities' struct of GL context constants is no longer a pointer but a static struct.
As usual, a tree of this series can be found at <https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v2>.
A tree of the larger WIP branch from which this series is taken: <https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>.
Thanks in advance for the reviews!
cheers,
Eduardo
spirv_extensions: rename nir_spirv_supported_extensions
mesa: move nir_spirv_supported_capabilities definition
i965: initialize SPIR-V capabilities
spirv_extensions: add GL_ARB_spirv_extensions boilerplate
spirv_extensions: add list of extensions and to_string method
spirv_extensions: define spirv_extensions_supported
spirv_extensions: add spirv_supported_extensions on gl_constants
spirv_extensions: i965: initialize SPIR-V extensions
nir/spirv: add gl_spirv_validation method
mesa/glspirv: Add struct gl_shader_spirv_data
mesa/glspirv: Add a _mesa_spirv_link_shaders() placeholder
mesa/program: Link SPIR-V shaders using the SPIR-V code-path
mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader
mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program
mesa/glspirv: Add a _mesa_spirv_to_nir() function
i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders
i965: Don't call process_glsl_ir() for SPIR-V shaders
mesa: Add boilerplate for the GL 4.6 alias of glSpecializeShaderARB
mesa: add GL_ARB_gl_spirv boilerplate
mesa/glspirv: Add struct gl_spirv_module
mesa: implement SPIR-V loading in glShaderBinary
mesa/shaderapi: add a getter for GL_SPIR_V_BINARY_ARB
mesa: refuse to compile SPIR-V shaders or link mixed shaders
mesa: add gl_constants::SpirVCapabilities
mesa: Implement glSpecializeShaderARB
src/amd/vulkan/radv_shader.c | 4 +-
src/compiler/Makefile.sources | 2 +
src/compiler/spirv/nir_spirv.h | 21 +-
src/compiler/spirv/spirv_extensions.c | 77 +++++++
src/compiler/spirv/spirv_extensions.h | 63 ++++++
src/compiler/spirv/spirv_to_nir.c | 160 +++++++++++++-
src/compiler/spirv/vtn_private.h | 2 +-
src/intel/vulkan/anv_pipeline.c | 4 +-
src/mapi/glapi/gen/ARB_gl_spirv.xml | 21 ++
src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 ++
src/mapi/glapi/gen/GL4x.xml | 11 +
src/mapi/glapi/gen/Makefile.am | 2 +
src/mapi/glapi/gen/gl_API.xml | 8 +
src/mapi/glapi/gen/gl_genexec.py | 1 +
src/mapi/glapi/gen/meson.build | 2 +
src/mesa/Makefile.sources | 4 +
src/mesa/drivers/dri/i965/brw_context.c | 26 +++
src/mesa/drivers/dri/i965/brw_link.cpp | 3 +-
src/mesa/drivers/dri/i965/brw_program.c | 14 +-
src/mesa/main/context.c | 2 +
src/mesa/main/extensions_table.h | 2 +
src/mesa/main/get.c | 7 +
src/mesa/main/get_hash_params.py | 3 +
src/mesa/main/getstring.c | 12 +
src/mesa/main/glspirv.c | 331 ++++++++++++++++++++++++++++
src/mesa/main/glspirv.h | 108 +++++++++
src/mesa/main/mtypes.h | 31 +++
src/mesa/main/shaderapi.c | 60 ++++-
src/mesa/main/shaderobj.c | 3 +
src/mesa/main/spirv_extensions.c | 60 +++++
src/mesa/main/spirv_extensions.h | 49 ++++
src/mesa/main/tests/dispatch_sanity.cpp | 3 +
src/mesa/meson.build | 4 +
src/mesa/program/ir_to_mesa.cpp | 23 +-
34 files changed, 1098 insertions(+), 38 deletions(-)
create mode 100644 src/compiler/spirv/spirv_extensions.c
create mode 100644 src/compiler/spirv/spirv_extensions.h
create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml
create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
create mode 100644 src/mesa/main/glspirv.c
create mode 100644 src/mesa/main/glspirv.h
create mode 100644 src/mesa/main/spirv_extensions.c
create mode 100644 src/mesa/main/spirv_extensions.h

_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev