Patches 1-2, 8-10, 14-19, 21 are Reviewed-by Karol Herbst <kher...@redhat.com>
For Patches 16-22 I looked inside your repository. regarding all Patches modifying the headers: shouldn't we just update those headers? I personally don't care much, because nobody will use mesa as the system OpenCL implementation and just go with ocl-icd or something more mature than clover anyway. On Tue, Jan 23, 2018 at 1:33 AM, Pierre Moreau <pierre.mor...@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? > > * 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. > > > (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. > > * 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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
