'Maintenance and support obligations' - the only maintenance obligations would be don't regress our tests when you change code, that is no different from doing changes to any other target. Chandler raised some pretty important points in his reply, and we will need to address them. In Sam's original email he did say 'We are open to the SelectionDAG/MC approach if the community recommends it.' <- I would infer from Chandler's email that he is most definitely recommending this, and if the community as a whole believes this is the direction we should take then we are happy to proceed in that way. Really though our 'proposal' is that we have code that does bi-way conversion from LLVM IR <-> SPIR-V, we are asking the community where we can place it in the tree that makes most sense. We believe that there will be real benefit for people to write their own shader/kernel languages using the awesome infrastructure that LLVM and Clang provides, and I for one want to enable the community to do just that.> I would be actively opposed to seeing changes in this direction land > without substaintial further discussion and active > contribution/ownership by an existing active member of the LLVM > community.We would be very happy for 'an existing active member of the LLVM community' to help us in our endeavours, but to 'actively oppose' the inclusion of our code based (aside from Chandler's concerns that I have already addressed) on a minor niggle with the data layout (which as Sam pointed out is as easy as using the data layout that exists at present for the SPIR/SPIR64 targets) seems rather harsh. The main issue of contention has been around 'is it a target'. I still think the most unobtrusive way for us to proceed here is to add it as a target that is only enabled via the experimental targets mechanism (like many new targets have been before) and then we can evolve the code from there. -Neil. On 22/05/15 06:34, Philip Reames wrote:> After following the thread through the rounds of discussion that have > happened, I have serious concern about this proposal and believe that > it should not land as proposed. I am not opposed to the idea of SPIR, > but the current proposal is lacking key details and could be construed > to imply maintenance and support obligations on the community as a > whole which are not clearly defined, much less widely discussed and > agreed to. > > I would suggest that interested parties need to settle on a usage > model, what the model implies for the rest of LLVM, and come back with > a more detailed specific proposal. > > From what I've gathered so far, it really sounds like there are two > proposals within one here. > > The first is a SPIR-frontend targeting arbitrary backends. The usage > model sounds like an existing SPIR program being compiled to either > x86 (say for testing purposes) or one of the existing GPU focused > backends. To me, this sounds like the less invasive part since none > of the concerns about data layout (or lack there of) apply. By the > time we're compiling a SPIR program to a particular target, we should > be able to optimize using target specific information. My suggestion > would be to establish a new repository (hosted on llvm.org) which > holds this frontend. I have seen no reason this should live in the > main llvm or clang repositories in the discussion to date. > > The second sounds like it's a proposal to have Clang support an SPIR > backend. Exactly what form this would take appears to be really > unclear and the implications for the project have not be spelled out. > In particular, it's not clear to me that preserving all the invariants > needed to emit SPIR through the optimizer is even possible with our > current IR and type system. I would be actively opposed to seeing > changes in this direction land without substaintial further discussion > and active contribution/ownership by an existing active member of the > LLVM community. > > Philip > > On 05/13/2015 05:56 AM, Liu, Yaxun (Sam) wrote: >> Khronos Group SPIR WG is working on a bi-way converter between LLVM >> bitcode and SPIR-V >> (https://www.khronos.org/registry/spir-v/specs/1.0/SPIRV.pdf ) binary >> and is willing to upstream it to the LLVM project. >> >> The bi-way converter uses a common in-memory representation of >> SPIR-V. It works by breaking down a module to instructions and >> construct the translated module in memory then output it. Currently >> it supports SPIR-V common instructions and OpenCL specific >> instructions. Supporting of other languages is under consideration. >> >> We plan to refactor the LLVM to SPIR-V converter as a backend at >> llvm/lib/Target/SPIRV to allow Clang targeting SPIR-V. Since this >> will result in an unconventional backend which does not use >> SelectionDAG/MC, we would like to know whether it is acceptable. We >> are open to the SelectionDAG/MC approach if the community recommends it. >> >> For the SPIR-V to LLVM converter, we are seeking suggestions on its >> proper location in the LLVM project. >> >> Any comments are welcome. Thanks. >> >> Yaxun Liu >> AMD >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Let me start by emphasizing that I only speak for myself. This is my opinion, and nothing more. On 05/22/2015 03:55 AM, Neil Henning wrote:> 'Maintenance and support obligations' - the only maintenance > obligations would be don't regress our tests when you change code, > that is no different from doing changes to any other target.I think SPIR as discussed is different in two potentially important ways: - The lack of target information. Can we even run IR passes over SPIR derived IR and get back valid SPIR? This hasn't been resolved. - SPIR has explicit compatibility provisions across versions. Adding a bi-directional converter makes the community responsible for preserving this compatibility, even in face of otherwise breaking IR changes. I haven't seen this clearly discussed yet.> > Chandler raised some pretty important points in his reply, and we will > need to address them. In Sam's original email he did say 'We are open > to the SelectionDAG/MC approach if the community recommends it.' <- I > would infer from Chandler's email that he is most definitely > recommending this, and if the community as a whole believes this is > the direction we should take then we are happy to proceed in that way. > > Really though our 'proposal' is that we have code that does bi-way > conversion from LLVM IR <-> SPIR-V, we are asking the community where > we can place it in the tree that makes most sense. We believe that > there will be real benefit for people to write their own shader/kernel > languages using the awesome infrastructure that LLVM and Clang > provides, and I for one want to enable the community to do just that.Like I said, I'm not opposed to the idea of SPIR. I'm not even opposed to supporting SPIR w/in LLVM. I am seriously concerned about the *current proposal*. I will also point out that I actively tried to suggest a path forward with incremental progress being made.> >> I would be actively opposed to seeing changes in this direction land >> without substaintial further discussion and active >> contribution/ownership by an existing active member of the LLVM >> community. > > We would be very happy for 'an existing active member of the LLVM > community' to help us in our endeavours,My reasoning here may not have been clear, and reading this out of context it sounds more dismissive than I'd intended. Sorry. Most of my concerns about the proposal come down to issues of process, community norms, and support. By working with one or more folks who have established histories in the community makes it easier to trust that those issues will be resolved properly. In particular, it makes it far less likely that the contribution will end up being a "code drop" w/o any ongoing active development.> but to 'actively oppose' the inclusion of our code based (aside from > Chandler's concerns that I have already addressed) on a minor niggle > with the data layout (which as Sam pointed out is as easy as using the > data layout that exists at present for the SPIR/SPIR64 targets) seems > rather harsh.I disagree. By actively oppose, all I mean is that I will argue against it in it's current form. I am simply one contributor to LLVM. If you convince other folks that the current approach is sufficient, my opposition doesn't matter to you in practice.> The main issue of contention has been around 'is it a target'. I still > think the most unobtrusive way for us to proceed here is to add it as > a target that is only enabled via the experimental targets mechanism > (like many new targets have been before) and then we can evolve the > code from there.At the moment, I don't see the "is it a target" as being the core concerns. The serialization, support, and data layout pieces are far more concerning to me personally.> > -Neil. > > On 22/05/15 06:34, Philip Reames wrote: >> After following the thread through the rounds of discussion that have >> happened, I have serious concern about this proposal and believe that >> it should not land as proposed. I am not opposed to the idea of >> SPIR, but the current proposal is lacking key details and could be >> construed to imply maintenance and support obligations on the >> community as a whole which are not clearly defined, much less widely >> discussed and agreed to. >> >> I would suggest that interested parties need to settle on a usage >> model, what the model implies for the rest of LLVM, and come back >> with a more detailed specific proposal. >> >> From what I've gathered so far, it really sounds like there are two >> proposals within one here. >> >> The first is a SPIR-frontend targeting arbitrary backends. The usage >> model sounds like an existing SPIR program being compiled to either >> x86 (say for testing purposes) or one of the existing GPU focused >> backends. To me, this sounds like the less invasive part since none >> of the concerns about data layout (or lack there of) apply. By the >> time we're compiling a SPIR program to a particular target, we should >> be able to optimize using target specific information. My suggestion >> would be to establish a new repository (hosted on llvm.org) which >> holds this frontend. I have seen no reason this should live in the >> main llvm or clang repositories in the discussion to date. >> >> The second sounds like it's a proposal to have Clang support an SPIR >> backend. Exactly what form this would take appears to be really >> unclear and the implications for the project have not be spelled >> out. In particular, it's not clear to me that preserving all the >> invariants needed to emit SPIR through the optimizer is even possible >> with our current IR and type system. I would be actively opposed to >> seeing changes in this direction land without substaintial further >> discussion and active contribution/ownership by an existing active >> member of the LLVM community. >> >> Philip >> >> On 05/13/2015 05:56 AM, Liu, Yaxun (Sam) wrote: >>> Khronos Group SPIR WG is working on a bi-way converter between LLVM >>> bitcode and SPIR-V >>> (https://www.khronos.org/registry/spir-v/specs/1.0/SPIRV.pdf ) >>> binary and is willing to upstream it to the LLVM project. >>> >>> The bi-way converter uses a common in-memory representation of >>> SPIR-V. It works by breaking down a module to instructions and >>> construct the translated module in memory then output it. Currently >>> it supports SPIR-V common instructions and OpenCL specific >>> instructions. Supporting of other languages is under consideration. >>> >>> We plan to refactor the LLVM to SPIR-V converter as a backend at >>> llvm/lib/Target/SPIRV to allow Clang targeting SPIR-V. Since this >>> will result in an unconventional backend which does not use >>> SelectionDAG/MC, we would like to know whether it is acceptable. We >>> are open to the SelectionDAG/MC approach if the community recommends >>> it. >>> >>> For the SPIR-V to LLVM converter, we are seeking suggestions on its >>> proper location in the LLVM project. >>> >>> Any comments are welcome. Thanks. >>> >>> Yaxun Liu >>> AMD >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Sorry for my late reply. Thanks for sharing your concerns. I commented about some technical concerns below. For the concerns about the ownership and long-term maintenance, I will bring them back to the SPIR WG. Hopefully we will come back with a satisfying solution. Sam -----Original Message----- From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On Behalf Of Philip Reames Sent: Friday, May 22, 2015 12:29 PM To: Neil Henning; llvmdev at cs.uiuc.edu Subject: Re: [LLVMdev] [RFC] Upstreaming LLVM/SPIR-V converter On 05/22/2015 03:55 AM, Neil Henning wrote:> 'Maintenance and support obligations' - the only maintenance > obligations would be don't regress our tests when you change code, > that is no different from doing changes to any other target.I think SPIR as discussed is different in two potentially important ways: - The lack of target information. Can we even run IR passes over SPIR derived IR and get back valid SPIR? This hasn't been resolved. + My understanding is that SPIR-V targets generic OpenCL/GL devices and assumes a common natural data layout. Optimizations allowed on SPIR-V should not introduce any target specific dependence. The common use case of SPIR-V is that a vendor generates SPIR-V with minor optimizations and sends it to a user. The user translates SPIR-V to LLVM and performs target-specific optimizations and codegen, and executes the ISA. - SPIR has explicit compatibility provisions across versions. Adding a bi-directional converter makes the community responsible for preserving this compatibility, even in face of otherwise breaking IR changes. I haven't seen this clearly discussed yet. + The current implementation of LLVM to SPIR-V converter works by breaking down the IR to instructions and constructing the corresponding entities in SPIR-V. The SPIR-V to LLVM converter works the other way around. Their dependence on the IR is mainly on the API for getting information about entities and creating entities in the IR. The dependence is not much different from other LLVM module passes. I have seen porting efforts of the converter among LLVM 3.2/3.4/3.6, which are no more difficult than typical LLVM module passes. For example, the main porting efforts from 3.4 to 3.6 is the change in API changes about metadata and user iterator, which are quite straightforward.
On Fri, May 22, 2015 at 9:29 AM, Philip Reames <listmail at philipreames.com> wrote:> Let me start by emphasizing that I only speak for myself. This is my > opinion, and nothing more. > > On 05/22/2015 03:55 AM, Neil Henning wrote: >> >> 'Maintenance and support obligations' - the only maintenance obligations >> would be don't regress our tests when you change code, that is no different >> from doing changes to any other target. > > I think SPIR as discussed is different in two potentially important ways: > - The lack of target information. Can we even run IR passes over SPIR > derived IR and get back valid SPIR? This hasn't been resolved. > - SPIR has explicit compatibility provisions across versions. Adding a > bi-directional converter makes the community responsible for preserving this > compatibility, even in face of otherwise breaking IR changes. I haven't > seen this clearly discussed yet.Also, "don't regress tests" is a really strong obligation to impose if "doing normal things to LLVM IR allowed by LLVM langref" may break SPIR tests, because SPIR has stricter requirements and you test them (and, if you don't test them, then the target is certain to break). So as Philip says, this definitely sounds like something that needs some real resolution.>> >> >> Chandler raised some pretty important points in his reply, and we will >> need to address them. In Sam's original email he did say 'We are open to the >> SelectionDAG/MC approach if the community recommends it.' <- I would infer >> from Chandler's email that he is most definitely recommending this, and if >> the community as a whole believes this is the direction we should take then >> we are happy to proceed in that way. >> >> Really though our 'proposal' is that we have code that does bi-way >> conversion from LLVM IR <-> SPIR-V,And if you test this, as Philip says, the llvm community becomes responsible for ensuring compatibility/figuring out ways to make this translation into SPIR (which uh, AFAICT, was developed in part because the llvm community didn't want to guarantee this :P). How do you propose that work, exactly (literally, i'm not suggesting it doesn't, i'm trying to understand the proposed moel)? Let me give a practical example: David Blaikie has been working on removing pointer types from some places and adding them in others, to remove requirements to bitcast pointers. This requires bitcode changes, and IR/langref changes. SPIR, even if it were to follow in the same path, does not in fact do this right now. In the LLVM bitcode/IR, one-way translation (from old bitcode to new) from old to new is easy. Two way translation from old to new is much harder (now he really has to have a pass to output bitcasts where they don't exist now to match the semantics of the old IR). But that is what is essentially going to need to be done. Thus, if you test the SPIR translation of LLVM load/stores/geps, you would be affected by these changes. Do you expect David to write that part of a conversion pass/or update any existing conversion pass to make these changes to not break your tests? If so, you are asking the LLVM community to take on the burden of supporting the SPIR serialization, essentially, and that will significantly change the ability of the llvm community to innovate LLVM's IR. If not, what is the process you expect to be followed here? IE do you expect him to break you, wait for SPIR to decide whether they want to do the same thing, etc?