Pierre Moreau <[email protected]> writes: > On 2018-02-07 — 12:36, Francisco Jerez wrote: >> Pierre Moreau <[email protected]> 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. >
I don't think you misunderstood, but it seemed so from your previous paragraph. ;) >> >> 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: Yes, there's always going to be overhead from multiple conversions in some cases (e.g. SPIR-V input on a driver that wants a native binary ultimately), but ideally the responsibility of converting back and forth would be handled by the state tracker instead of by the driver. > 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. Why would we ever have to do either of those? If the driver wants SPIR-V or NIR as IR, a SPIR-V input doesn't need to be translated to LLVM IR in the first place. Likewise if the driver wants a native binary for an input in CL C, it doesn't need to be converted to SPIR-V in the first place. > 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
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
