[Mesa-dev] [PATCH v2 00/22] Introducing SPIR-V support to clover

[Francisco Jerez]

Pierre Moreau <pierre.morrow at free.fr> writes:

> On 2018-01-24 — 09:19, Tomeu Vizoso wrote:
>> On 01/24/2018 12:03 AM, Karol Herbst wrote:
>> > On Tue, Jan 23, 2018 at 11:46 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>> > > Pierre Moreau <pierre.morrow at free.fr> writes:
>> > > 
>> > > > On 2018-01-23 — 14:02, Francisco Jerez wrote:
>> > > > > Karol Herbst <kherbst at redhat.com> writes:
>> > > > > 
>> > > > > > there seem to be some patches missing?
>> > > > > > 
>> > > > > > On Tue, Jan 23, 2018 at 1:33 AM, Pierre Moreau <pierre.morrow at free.fr> wrote:
>> > > > > > > 
>> > > > > > > * 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.
>> > > > > > 
>> > > > > 
>> > > > > I think it would be more straightforward to compile source programs into
>> > > > > SPIRV if the driver supports it (or if it supports any other IR that
>> > > > > could possibly be translated from SPIRV after link time, e.g. NIR or
>> > > > > maybe even TGSI).  That means that there is a single canonical IR for
>> > > > > each CL device and we don't need to deal with linking different
>> > > > > combinations of IRs together.  If the driver doesn't support SPIRV nor
>> > > > > any of the IRs derived from it, it better support LLVM IR instead, so we
>> > > > > can just use that as canonical IR within the state tracker, and possibly
>> > > > > accept the same representation as input to clCreateProgramWithIL()
>> > > > > instead of SPIRV.
>> > > > 
>> > > > “On top of” SPIR-V, not “instead of”, as SPIR-V is the only IL which is
>> > > > mandatory to support, according to the specification.
>> > > 
>> > > That's right, but it just means that devices that have LLVM as canonical
>> > > IR don't get support for cl_khr_il_program for the time being, until
>> > > Khronos' SPIRV-to-LLVM converter gets upstreamed.
>> > > 
>> > 
>> > we could use tomeus out of tree llvm-spirv module though, but this
>> > would also need some maintenance. It would be a better solution than
>> > using that llvm-spirv fork from khronos
>> 
>> Though I still cannot commit at the moment to maintain it, there's so many
>> people whose plans could benefit from it, that maybe it won't be such a
>> problem to maintain such a "packagable" fork until it gets merged in LLVM
>> proper.
>> 
>> Besides Mesa, there's interest from compiler writers such as D and Volt.
>
> I am currently working on the v3 of this series, and I am split between:
> 1) dropping all the plumbing between clCreateProgramWithIL and the rest of
>    clover, i.e. you can create a program using that function, but it will refuse
>    to compile and link; compiling and linking support, along with using a
>    canonical IR in clover would come in a second pull request once llvm-spirv
>    is in a packageable form.
> 2) update clover to use a canonical IR in this series, even if that means the
>    series won’t be mergeable until llvm-spirv is in a packageable form, and we
>    have some way to convert SPIR-V or LLVM IR to TGSI.
>
> Point 2) would be the best option, as it also gives the opportunity to actually
> test the code; currently one needs non-upstream code, either for an updated
> spirv_to_nir which handles OpenCL SPIR-V, or a direct SPIR-V consumer, to be
> able to test this series.
>
I wouldn't bother to do 1) unless there is some immediate benefit from
having a dysfunctional implementation of clCreateProgramWithIL merged
upstream in the meantime.

> Francisco: Do you know of anyone using the TGSI backend of clover? I was
> wondering if it could be dropped or not.
>

No.  Feel free to delete it.

> Regards,
> Pierre
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180127/f025a652/attachment-0001.sig>