[Mesa-dev] [PATCH v2 00/22] Introducing SPIR-V support to clover
Karol Herbst
kherbst at redhat.com
Tue Jan 23 07:20:04 UTC 2018
there seem to be some patches missing?
On Tue, Jan 23, 2018 at 1:33 AM, Pierre Moreau <pierre.morrow at free.fr> wrote:
> Hello,
>
> Here is the second version of my initial series for adding SPIR-V support to
> clover, after the RFC back in May 2017.
>
> For recap, the focus of this series is to let clover accept SPIR-V binaries
> through the cl_khr_il_program extension (from OpenCL 1.2 and on), as well as
> through some core features (since OpenCL 2.1). Even if OpenCL 2.1 support in
> clover is some way off, there is another motivation for supporting SPIR-V in
> clover, as multiple drivers are interested in adding OpenCL support by
> converting SPIR-V to NIR.
>
> Note: the series is based on master + Karol’s patch “clover: add functions up
> to 2.2 to ICD dispatch table”.
>
>
> The various patches can be split in different categories:
>
> * Patches 1 through 7: some clover clean-up, adding and moving some
> functionalities around to make the implementation easier in the rest of the
> series.
>
> * Patches 8 through 13: define SPIR-V as a new IR, add a new frontend to clover
> to deal with SPIR-V, and edit compile and link operations to handle SPIR-V as
> well.
>
> * Patches 14 through 19: implement cl_khr_il_program
>
> * Patches 20 through 22: implement OpenCL 2.1 support on top of
> cl_khr_il_program
>
>
> Changes since the RFC
> ---------------------
>
> * Most SPIR-V utilities were dropped, and the remaining ones have been moved to
> the clover SPIR-V frontend rather than sitting in src/gallium/auxiliary/spirv.
>
> * The SPIR-V linker has been completely dropped from this series and instead
> merge in SPIRV-Tools [1].
>
> * Since SPIRV-Tools now exports a pkgconfig .pc file, use it for detecting the
> library.
>
> * Integrate the series with Meson.
>
> * Use pipe_llvm_program_header to pass in the size of the SPIR-V module, rather
> than adding a new attribute to pipe_compute_state, as suggested by Francisco
> Jerez.
>
> * Check that the device supports the capabilities defined in the SPIR-V binary.
>
> * Check that the platform/device supports the extensions used in the SPIR-V
> binary.
>
> * Fix the implementation responsible for filling up the symbols of the clover
> module based on the input SPIR-V binary.
>
> * No longer raw SPIR-V binaries through clCreateProgramWithBinary, but instead
> keep the current de-serialisation of the clover module, which may contain a
> SPIR-V binary.
>
> * Track whether a library was created with the --enable-link-options flag or
> not. This is currently not useful as the linker ignores most link options,
> but it will become useful when the linker handles those options.
>
> * Implement cl_khr_il_program.
>
> * Most of patches 1 through 8 (apart from patch 2).
>
>
> Discussions
> -----------
>
> * Before, when linking different modules together, you knew that all modules
> would use the same IR, as all were created using clCreateProgramWithSource,
> therefore the linker could just call the linking function corresponding to
> the target’s preferred IR. But with the introduction of
> clCreateProgramWithIL(KHR)?, we can now end up in a case where we try to link
> a module using NIR as IR (created through clCreateProgramWithSource, assuming
> that is the driver’s preferred IR), with another module using SPIR-V as IR
> (created through clCreateProgramWithIL). How do we handle such a case: should
> we translate the SPIR-V to NIR and use a NIR linker on them, or convert NIR
> to SPIR-V and use the SPIR-V linker? NIR and LLVM IR can be handled
> relatively easily, but what about TGSI?
>
I think we will never be able to convert all IRs into any other IR, so
that I would suggest to leave those IRs unconverted until they get
linked together and there the code can decide on a common IR for
linking. So if we get source code, we can parse it to llvm IR and
leave it like that until it gets linked. Converting back and forth
would require us to write all those conversion paths and I am assume
this wouldn't be worth the trouble.
> * In that regard, is anyone using the TGSI frontend in clover? If not, is
> anyone planning to use it? And if still not, shouldn’t we just remove it?
>
> * In the same vein as the linking discussion just above, what should happen
> when the driver’s preferred IR is one of the IRs not currently supported by
> clover, like NIR for example? Should `compile()` generate a SPIR-V binary
> which is directly translated to NIR, or should we keep everything in SPIR-V
> until the very last moment, right before sending the IR to the driver? If
> all the drivers supporting compute through clover support an IR that can be
> translated from SPIR-V, it might be easier to keep everything inside clover
> as SPIR-V binaries, until we need to pass the program to the driver, in which
> case we convert it on the fly.
>
don't rely on the preferred IR inside clover. It should only matter
_after_ linking. If we can't link to the preferred IR, but to a
supported one, that's fine as well. I am sure that over the time where
we actually start supporting multiple IRs we would run into troubles
if we strictly rely on the preferred one. Or we have to do some
fundamental changes on how to figure out the preferred IR, like a
driver wants to prefer something else if it is a OpenCL shader instead
of an OpenGL compute shader?
Also using SPIR-V as the common IR might be not what the AMD driver
wants to do, because they just want to get straight to LLVM -> native.
>
> (Still) missing
> ---------------
>
> * As there is no upstream version of LLVM which can produce SPIR-V out of
> OpenCL code, clCreateProgramWithSource will refuse to work if the target’s
> preferred IR is SPIR-V, for now.
>
here again, please don't rely too much on the preferred IR. Most
drivers will either have NIR, LLVM or TGSI set there. So to force
SPIR-V you are already out of luck and introduce some level of pain
for drivers to actually handle this situation. In general we need a
better way to figure out what IR to actually use for the driver.
In the end we should never fail to compile/link just because we can't
produce the preferred IR. If the driver can't handle any of the
supported IRs the driver either have to fix it or stop exposing
support for it.
> * Optimisation linking options are ignored for now as SPIRV-Tools’ linker does
> not supported them yet.
>
>
> Thank you in advance for reviewing/commenting,
> Pierre
>
> [1]: https://github.com/KhronosGroup/SPIRV-Tools/
>
>
> Pierre Moreau (22):
> clover/api: Fix tab indentation to spaces
> clover: Add additional functions to query supported IRs
> clover/api: Fail if trying to build a non-executable binary
> clover: Disallow creating libraries from other libraries
> clover: Track flags per module section
> clover: Move device extensions definitions to core/device.cpp
> clover: Move platform extensions definitions to clover/platform.cpp
> include/pipe: Define SPIRV as an IR
> configure.ac,meson: Check for SPIRV-Tools
> clover/spirv: Import spirv.hpp11 version 1.0 (rev 12)
> clover/spirv: Add functions for parsing arguments, linking programs,
> etc.
> clover: Refuse to compile source code to SPIR-V
> clover: Handle the case when linking SPIR-V binaries together
> clover: Add a pointer property to return ILs
> include/CL: Add cl_khr_il_program
> clover: Implement clCreateProgramWithILKHR
> clover: Handle CL_PROGRAM_IL_KHR in clGetProgramInfo
> clover/api: Implement CL_DEVICE_IL_VERSION_KHR
> clover: Advertise cl_khr_il_program
> include/CL: Export OpenCL 2.1 functions
> clover: Implement clCreateProgramWithIL from OpenCL 2.1
> clover: Use OpenCL 2.1 defines in place of cl_khr_il_program
>
> configure.ac | 5 +
> include/CL/cl.h | 8 +
> include/CL/cl_ext.h | 34 +
> include/CL/cl_platform.h | 1 +
> meson.build | 2 +
> src/gallium/include/pipe/p_defines.h | 1 +
> src/gallium/state_trackers/clover/Makefile.am | 15 +-
> src/gallium/state_trackers/clover/Makefile.sources | 4 +
> src/gallium/state_trackers/clover/api/device.cpp | 16 +-
> src/gallium/state_trackers/clover/api/dispatch.cpp | 2 +-
> src/gallium/state_trackers/clover/api/dispatch.hpp | 4 +
> src/gallium/state_trackers/clover/api/platform.cpp | 6 +-
> src/gallium/state_trackers/clover/api/program.cpp | 70 +-
> src/gallium/state_trackers/clover/core/device.cpp | 26 +
> src/gallium/state_trackers/clover/core/device.hpp | 4 +
> src/gallium/state_trackers/clover/core/module.cpp | 1 +
> src/gallium/state_trackers/clover/core/module.hpp | 13 +-
> .../state_trackers/clover/core/platform.cpp | 5 +
> .../state_trackers/clover/core/platform.hpp | 2 +
> src/gallium/state_trackers/clover/core/program.cpp | 94 +-
> src/gallium/state_trackers/clover/core/program.hpp | 14 +
> .../state_trackers/clover/core/property.hpp | 39 +
> .../state_trackers/clover/llvm/codegen/bitcode.cpp | 3 +-
> .../state_trackers/clover/llvm/codegen/common.cpp | 2 +-
> src/gallium/state_trackers/clover/meson.build | 10 +-
> .../state_trackers/clover/spirv/invocation.cpp | 668 ++++++++++++++
> .../state_trackers/clover/spirv/invocation.hpp | 54 ++
> .../state_trackers/clover/spirv/spirv.hpp11 | 997 +++++++++++++++++++++
> .../state_trackers/clover/tgsi/compiler.cpp | 3 +-
> 29 files changed, 2067 insertions(+), 36 deletions(-)
> create mode 100644 src/gallium/state_trackers/clover/spirv/invocation.cpp
> create mode 100644 src/gallium/state_trackers/clover/spirv/invocation.hpp
> create mode 100644 src/gallium/state_trackers/clover/spirv/spirv.hpp11
>
> --
> 2.16.0
>
More information about the mesa-dev
mailing list