[Mesa-dev] [PATCH 11/11] r600/radeonsi/clover: always assume PIPE_SHADER_IR_NATIVE for clover

[Pierre Moreau]

On 2018-02-07 — 12:36, Francisco Jerez wrote:
> Pierre Moreau <pierre.morrow at free.fr> writes:
> 
> > On 2018-02-06 — 20:50, Jan Vesely wrote:
> > [snip]
> >> > > Happy to here suggestions for solving the current conflict in uses of
> >> > > PIPE_SHADER_CAP_PREFERRED_IR.
> >> > 
> >> > One option could be to:
> >> > * look at the preferred IR
> >> >   |-> if clover supports it, use it
> >> >   |-> else, check if any IR supported by clover are supported by the driver,
> >> >       and pick the first one that works
> >> > 
> >> > Also, clover will be switching (experimentally first) to using SPIR-V as the
> >> > canonical IR, for all the compiling and linking internally, before translating
> >> > the resulting executable to a representation the driver can handle.
> >> 
> >> Why? what is this good for? Is the expected path for radeonsi:
> >>  clc->llvm->spirv->nir->llvm->isa, or clc->llvm->spirv->llvm->isa ?
> >
> > It depends how we end up deciding which IR to feed the driver (using
> > *_PREFERRED_IR, *_SUPPORTED_IRS, or something else), but the path would be
> > clc->llvm->spirv->*->driver or spirv->*->driver depending on how the programs
> > are created (resp. clCreateProgramWithSource or clCreateProgramWithIL).
> >
> 
> I thought we had agreed to continue supporting the current "clc->llvm
> ir->native binary->driver" path for targets that have an LLVM back-end
> (e.g. AMDGPU), so we can just give the pipe driver a binary, and also
> allow the application to query and cache programs as native machine
> code?

Sorry, I must have misunderstood you then. I thought we had agreed to use
either SPIR-V or LLVM IR as the “canonical” IR, i.e. we would compile to and
link using that IR, and then convert to whatever IR the driver supports. So for
example, we compile to SPIR-V, link the different SPIR-V modules together, and
if we are creating an executable, we convert the linked module to native
machine code and store that in the clover module under the executable section.
That way, the driver receives the native code, and when querying for binaries,
the native code is returned (or whatever IR the driver supports).
For libraries, we want to keep them in SPIR-V/LLVM IR, as they will be used in
a linking step at some point.

> >> What advantages does it bring to any target? why can't the targets that
> >> are not natively supported by llvm just consume llvm IR and do the
> >> necessary translations on driver side?
> >
> > What disadvantages does it bring to have clover take care of the translation
> > (for IRs that are easily translatable from LLVM IR/SPIR-V)? It already has all
> > the dependencies needed, the translation is implemented in a single common
> > place, and if you don’t want OpenCL you do not need to still depend on LLVM
> > (except for the drivers for AMD cards).
> >
> 
> Maybe that the translation is unnecessary and is going to create more
> than zero work for the pipe driver to obtain a binary?

If the translation is unnecessary (I am talking about the translation happening
on the executable, before it is passed to the driver), then that IR should be
part of PIPE_SHADER_CAP_SUPPORTED_IRS.
Unless we decide to go with using LLVM IR in some cases and SPIR-V in other
cases, akin to what we have currently between LLVM IR and TGSI, there will
always be some overhead: either converting a SPIR-V input to LLVM IR to later
go back via SPIR-V to NIR, or go via LLVM IR to SPIR-V before going back to
LLVM IR.
I thought the plan was to either use LLVM IR or SPIR-V for everyone (as all current
consumers, and future ones via NIR, can be handled from those two IRs), but if
that’s not the case, then I am fine with that as well.

> >> > So once
> >> > spirv_to_nir supports OpenCL SPIR-V (which I believe is being worked on by Rob
> >> > Clark for freedreno, and Karol Herbst for Nouveau), RadeonSI could start
> >> > accepting even OpenCL kernels as NIR.
> >> 
> >> Why would anyone want that?
> >
> > In case it simplifies the code in the driver; I haven’t looked at that code
> > though. It was only a suggestion, and meant that for RadeonSI, there could no
> > longer be a conflict between the IR fed from OpenCL and the one from OpenGL,
> > which was one of the problem addressed in this patch.
> >
> 
> As long as we use PIPE_SHADER_CAP_SUPPORTED_IRS to enumerate the IRs the
> driver is willing to accept I don't think there is a real conflict, the
> pipe driver can just set both bits and have the state tracker choose
> which one to provide.

Sure.

> >> what is the expected path from CLC? if you
> >> need the external spirv tools package, why not use it in the other
> >> direction and keep LLVM IR as clover preferred representation?
> >> note that libclc is distributed in target specific LLVM IR.
> >
> > I am not entirely sure what the interaction with libclc should be, as I am only
> > starting to look into it. Possibly we would add a spir[64] target to it and use
> > that.
> >
> 
> Yeah, I think having a spir-v target-(un)specific implementation of
> libclc would be a pretty reasonable plan.

I’ll most likely need some help/pointers on that, but I’ll do that in a
separate thread.

> > We could indeed use LLVM IR instead of SPIR-V, as mentioned by Francisco Jerez
> > in his reply to “[PATCH v2 00/22] Introducing SPIR-V support to clover” on the
> > 23rd of January 2018 (I don’t have a direct link as this is one of the emails
> > which did not get archived), which shouldn’t change much apart from having:
> >
> > * clc->(compile)->llvm->(link)->llvm->(translate to driver IR)
> > * spirv->llvm->(link)->llvm->(translate to driver IR)
> >
> > instead of
> >
> > * clc->(compile)->spirv->(link)->spirv->(translate to driver IR)
> > * spirv->(link)->spirv->(translate to driver IR)
> >
> > (For drivers using NIR, the translation would be either llvm->spirv->nir or
> > spirv->nir; I don’t think there is a llvm->nir pass, but I could be mistaken.)
> >
> > However, with OpenCL 2.2 you can specialise constants, so if we use LLVM IR, we
> > would either need to
> > 1. keep a hash map from SPIR-V IDs to LLVM IR variables/functions and edit the
> >    LLVM module;
> > 2. convert back to SPIR-V (and hope that the SPIR-V IDs didn’t change),
> >    specialise, and convert back again to LLVM IR.
> > Or, if we use SPIR-V, we just do the specialisation directly.
> >
> 
> I don't think constant specialization changes the picture at all, AFAIUI
> they only apply to kernels that are written in SPIR-V in the first
> place, and require a rebuild to take effect, so we could handle them in
> exactly the same way regardless of whether the SPIR-V is being
> translated into LLVM IR during compilation or whether it's being used as
> program representation internally by clover.

I had forgotten binaries fed to clCreateProgramWithIL() would only be
converted, if necessary, during the compilation phase, so indeed the constant
specialisation has no impact.

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