Hi, This is a heads-up that I'm preparing a v2 of this series incorporating all the feedback received so far.
I plan to submit it imminently (tomorrow?) and several patches have diverged quite a bit; so please hold on for a few hours on any review you might have planned. Thanks! Eduardo On 11/15/2017 02:22 PM, Eduardo Lima Mitev wrote: > Hello, > > This series is the first chunk of our on-going work to support > GL_ARB_gl_spirv [1] and GL_ARB_spirv_extensions [2] in Mesa and i965. It adds > API entry-points, API error checking, most of driver-agnostic setup and > housekeeping, and a few basic i965 bits. It is based off initial work that > Nicolai Hähnle did back in July. > > In the case of GL_ARB_spirv_extensions, we consider our work on this > extension to be completed. > > For GL_ARB_gl_spirv, this is the high level explanation of what we do: > > * When the user calls glShaderBinary() with a SPIR-V binary, we mark the > shader as a SPIR-V shader and save the binary module within it. > > * Upon a call to glSpecializeShaderARB(), we perform a pass over the saved > binary module to validate that the given specialization constants and entry > point are ok. For this, we use a modified version of spirv_to_nir() that > doesn't generate any NIR, but simply performs the validation we need. If > everything went ok, we then save the specialization constants and entry point > within the shader object as well. > > * Upon program linking, and before calling driver-specific LinkShader(), we > generate gl_linked_shader objects and associate them with the SPIR-V data > that has been recorded in gl_shader (namely: the binary module, the > specialization constants and the entry point name). > > * We also introduce a convenient spirv_to_nir() wrapper for backends that use > NIR. > > This is all this series does. At this point, driver-specific linkers have all > the SPIR-V information they need (attached to the gl_shader and > gl_linked_shader objects of a program) to implement whatever linking strategy > they choose. For i965, we are planning to go for a (mostly) pure NIR linking > approach (see below). > > > This is the series' logical break down: > > - Patch 1 introduces the extension in Mesa and adds 'mesa/main/glspirv.c/h' > files to hold its implementation. > > - Patch 2 adds a flag to gl_shader to tell SPIR-V and GLSL shaders apart. > > - Patch 3 adds error checks introduced by the extension. > > - Patches 4 to 6 make glShaderBinary() load SPIR-V, associating the SPIR-V > binary with the shader for later use. > > - Patches 7 to 9 add SPIR-V capabilities to a GL context. This information is > an extension requirement. In our case, we already fill it up for i965. > > - Patches 10 to 14 implement GL_ARB_spirv_extensions. > > - Patches 15 and 16 implement glSpecializeShaderARB(). > > - Patches 17 to 22 fork the linking code for GLSL or SPIR-V programs, to > avoid using the GLSL linker on SPIR-V shaders. The GLSL linker expects > GLSL-IR to be present, and that's NULL for SPIR-V shaders. > > - Patches 23 and 24 do the same as above but for i965, plus also generate the > NIR out of each SPIR-V shader. > > > A tree of this series can be found at > <https://github.com/Igalia/mesa/commits/arb_gl_spirv-series1-v1>. > > A tree of the larger branch from which this series is taken can be found at > <https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv>. Note that > this is still very experimental and will change a lot constantly. > > > What's our plan going forward? > > In Mesa we have a single linker that is heavily interwined with GLSL-IR. So, > if we want to link SPIR-V shaders, we have basically two approaches: > > 1. Convert the SPIR-V to GLSL-IR and reuse the existing linker: Introduces > yet another conversion in the shader compilation process, but allows us to > reuse a battle-tested linker. > > 2. Write a new linker: Obviously a lot of work. > > Or use a combination of the two approaches, a mixed (or incremental) solution > where we aim at writing a new linker, but for specific cases and/or > temporarily we generate some minimal GLSL-IR that would allow us to reuse > hard-to-rewrite parts of the existing GLSL linker. > > After spending weeks prototyping different solutions, we have more or less > settled for a mixed approach: write a pure NIR linker from scratch, that will > reuse parts of the GLSL linker, but trying to avoid generating any GLSL-IR to > feed the GLSL linker directly (doing that only when really needed). > > We have prototyped some bits of this NIR linker already with a promising > degree of success. At this point, we are confident that with this mixed > approach we can write clean, efficient linking code; while at the same time > have the flexibility to use the current GLSL linker to fill gaps. > > If you are interested in more details about our approach, we encourage you to > check the snapshot of our development branch at [3]. We would really > appreaciate early feedback! (don't forget to > MESA_EXTENSION_OVERRIDE="GL_ARB_gl_spirv"). > > Another deciding factor was recent discussions in Mesa, which suggests that a > NIR linker can be useful to other backends, or for other purposes such as a > later-stage shader cache. There is also the work Timothy started, which > aligns well and complements what we need for GL_ARB_gl_spirv. > > Another important aspect is testing, and here the work Ian is doing to > generate SPIR-V binaries from GLSL is crucial. It will allow us to feed the > NIR linker with a much larger set of shaders. We look forward to incorporate > this conversion in our code-path and development workflow. > > There is obviously a lot of work ahead to have a NIR linker at the same level > of quality as the GLSL linker. Ideally, this would be a joint effort by those > in the community who see value in having a pure NIR linker. > > Comments? > Thanks for reviewing! > > Eduardo > > [1] https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_gl_spirv.txt > [2] > https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_spirv_extensions.txt > [3] https://github.com/Igalia/mesa/commits/wip/igalia/arb_gl_spirv > > > Alejandro Piñeiro (8): > spirv_extensions: rename nir_spirv_supported_extensions > 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 (9): > mesa/glspirv: Add struct gl_shader_spirv_data > mesa/main: Add a 'spirv' flag to gl_shader_program_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 > > Nicolai Hähnle (7): > mesa: add GL_ARB_gl_spirv boilerplate > mesa: add gl_shader::SpirVBinary and getter > mesa: refuse to compile SPIR-V shaders or link mixed shaders > mesa/glspirv: Add struct gl_spirv_module > mesa: implement SPIR-V loading in glShaderBinary > 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 | 11 +- > src/compiler/spirv/spirv_extensions.c | 90 ++++++++ > src/compiler/spirv/spirv_extensions.h | 81 +++++++ > 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/Makefile.am | 2 + > src/mapi/glapi/gen/gl_API.xml | 6 + > src/mapi/glapi/gen/gl_genexec.py | 1 + > src/mesa/Makefile.sources | 4 + > src/mesa/drivers/dri/i965/brw_context.c | 29 +++ > src/mesa/drivers/dri/i965/brw_link.cpp | 3 +- > src/mesa/drivers/dri/i965/brw_program.c | 9 +- > src/mesa/main/context.c | 3 + > src/mesa/main/extensions_table.h | 2 + > src/mesa/main/get.c | 6 + > src/mesa/main/get_hash_params.py | 3 + > src/mesa/main/getstring.c | 7 + > src/mesa/main/glspirv.c | 345 > ++++++++++++++++++++++++++++ > src/mesa/main/glspirv.h | 108 +++++++++ > src/mesa/main/mtypes.h | 27 +++ > src/mesa/main/shaderapi.c | 56 ++++- > src/mesa/main/shaderobj.c | 3 + > src/mesa/main/spirvextensions.c | 60 +++++ > src/mesa/main/spirvextensions.h | 49 ++++ > src/mesa/main/tests/dispatch_sanity.cpp | 3 + > src/mesa/program/ir_to_mesa.cpp | 24 +- > 31 files changed, 1111 insertions(+), 27 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/spirvextensions.c > create mode 100644 src/mesa/main/spirvextensions.h > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
