>Right, what I was trying to say is that there are more benefits from>having this not be a target than there is from having it be a target.Please enumerate them, I have seen none posted so far . The implied “it is what all the the other backends do” w.r.t ISel/MC is at best(worst?) an implementation detail, and I’m still not quite sure why Chandler was so adamant about that. He seemed to imply that generating straight from the IR (as opposed to post legalisation?) introduces a direct dependance in the IR that the rest of LLVM would then be required to not break? I agree that the SPIRV backend should be insulated from changes the IR, although I’m not sure how to achieve that property. I’m also not sure how much, if at all, it would be susceptible to that to begin with. Deletions of instructions/attributes would obviously cause breakage and additions may cause unhandled and/or invalid combinations. I still don’t get the severity if this though, insight appreciated.>What you are proposing is a lot of work, and even if it does help to >avoid some duplicate work (which to be honest, I still don't quite >understand what duplication there would be if it weren't a target), >I don't think that is enough to justify the effort required.My point is that (modulo metadata, which I am still investigating better alternatives, and calling conventions) if SPIRV is a target, then a producer need not change their compilation pipeline at all to target SPIRV. There should be no effort required, it would come as a property of being a target. I think we are confusing each other again :( Leaving that aside for a moment, there are a number of advantages/requirements that, correct me if I’m wrong, would be impossible without a proper target. * Most critically: Intrinsics. I am almost certain that you would not accept the current mangling hacks, and if I am to support windows neither can I. Any solution would therefore need to be able to register intrinsics and I believe this is impossible without a target (and even if it is, it makes less sense than a target that doesn’t use ISel/MC). Not being able to use intrinsics is a complete deal breaker. *Basic optimisations (basic CSE,DCE,inlining): requires a TargetLibraryInfoImpl(?) which I believe requires a target. While not strictly necessary it would improve the readability of the resulting IR/SPIRV. All of the more complex optimisations would be done “post ingestion” of the SPIRV and with a different target triple so are unaffected by any decision made. See my reply to Hongbin for an approach. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170503/d26ad405/attachment.html>
On 05/03/2017 11:19 AM, Nicholas Wilson wrote:>>Right, what I was trying to say is that there are more benefits from > >>having this not be a target than there is from having it be a target. > > Please enumerate them, I have seen none posted so far . The implied “it is what all the the other backends do” w.r.t ISel/MC is at best(worst?) an implementation detail, and I’m still not quite sure why Chandler was so adamant about that. He seemed to imply that generating straight from the IR (as opposed to post legalisation?) introduces a direct dependance in the IR that the rest of LLVM would then be required to not break? I agree that the SPIRV backend should be insulated from changes the IR, although I’m not sure how to achieve that property. I’m also not sure how much, if at all, it would be susceptible to that to begin with. Deletions of instructions/attributes would obviously cause breakage and additions may cause unhandled and/or invalid combinations. I still don’t get the severity if this though, insight appreciated. >So there are really two questions here: 1. Should targets be required to use SelectionDAG/GlobalISEL ? 2. Should SPIR-V use SelectionDAG/GlobalISel? In my opinion, regardless of the answer to question #1, the answer to question #2 is no, SPIR-V should not use SelectionDAG/GlobalISel. I touched on this before in previous emails, but the main problem is that SelectionDAG (and GlobalISel to a lesser extent) plus the whole MachineInstr layer is a much lower-level representation than SPIR-V, so you will need to do a lot of extra work and/or modifications to existing infrastructure in order to get a working target, and even then you may be limited to emitting poor quality SPIR-V that other backends will have a hard time optimizing. With all this work, what advantages are you getting? If the only reason to do it this way is so you can use intrinsics, or TargetLibraryInfo, or easier integration with other tools, I think it would be better to try to save the effort and try to solve those problems in some other way. LLVM IR -> SPIR-V directly will give you better code, lower compile times. It will be more simple and easier to maintain, and you will be able to re-use existing SPIR-V parsers/writers that exist in SPIRV-Tools. This goes back to something I mentioned in my original email, but I really think the best thing to do for this project right now is to keep it separate from LLVM, clean up the code, and try to get people using it. It's going to be much easier to get this upstream in LLVM or even convince people that the answer to question #1 should be 'no' if we have a code base that is mature, well supported, and has a healthy userbase.> >>What you are proposing is a lot of work, and even if it does help to >>avoid some duplicate work (which to be honest, I still don't quite >>understand what duplication there would be if it weren't a target), >>I don't think that is enough to justify the effort required. > > My point is that (modulo metadata, which I am still investigating better alternatives, and calling conventions) if SPIRV is a target, then a producer need not change their compilation pipeline /at all/ to target SPIRV. There should be no effort required, it would come as a property of being a target. I think we are confusing each other again :( > > Leaving that aside for a moment, there are a number of advantages/requirements that, correct me if I’m wrong, would be impossible without a proper target. > > * Most critically: Intrinsics. I am almost certain that you would not accept the current mangling hacks, and if I am to support windows neither can I. Any solution would therefore need to be able to register intrinsics and I believe this is impossible without a target (and even if it is, it makes less sense than a target that doesn’t use ISel/MC). Not being able to use intrinsics is a complete deal breaker. >You don't need to register intrinsics to be able to use them, and it's also possible to register them without a backend, but this has not been done before.> *Basic optimisations (basic CSE,DCE,inlining): requires a TargetLibraryInfoImpl(?) which I believe requires a target. While not strictly necessary it would improve the readability of the resulting IR/SPIRV. All of the more complex optimisations would be done “post ingestion” of the SPIRV and with a different target triple so are unaffected by any decision made. See my reply to Hongbin for an approach.You don't need a target for this. TargeLibraryInfo is constructed based on the triple. -Tom
>So there are really two questions here:>1. Should targets be required to use SelectionDAG/GlobalISEL ? >2. Should SPIR-V use SelectionDAG/GlobalISel? > >In my opinion, regardless of the answer to question #1, the answer >to question #2 is no, SPIR-V should not use SelectionDAG/GlobalISel.Yes I have come the same conclusion when I saw how much type information was lost.>I touched on this before in previous emails, but the main problem is that >SelectionDAG (and GlobalISel to a lesser extent) plus the whole MachineInstr >layer is a much lower-level representation than SPIR-V, so you will >need to do a lot of extra work and/or modifications to existing >infrastructure in order to get a working target, and even then >you may be limited to emitting poor quality SPIR-V that other >backends will have a hard time optimizing. > >With all this work, what advantages are you getting? If the >only reason to do it this way is so you can use intrinsics, >or TargetLibraryInfo, or easier integration with other tools, >I think it would be better to try to save the effort and try >to solve those problems in some other way.I didn't really want to do that in the first place, but your responses and Chandler's reply in the previous RFC made it seem an absolute requirement.>LLVM IR -> SPIR-V directly will give you better code, lower compile >times. It will be more simple and easier to maintain, and you will >be able to re-use existing SPIR-V parsers/writers that exist >in SPIRV-Tools.Yes the current approach seems to be the place that has the most type info. (aside the AST of the producer, but that is a non sequitur, because what would we be building a backend for?). The parser and writer already exist in the code base, as do the interconversions between LLVM <->SPIR-V, so the only use would be for the verifier for testing. I mean I could replace them, but that is effort for nothing>This goes back to something I mentioned in my original email, but >I really think the best thing to do for this project right now is to >keep it separate from LLVM, clean up the code, and try to get people >using it. It's going to be much easier to get this upstream in LLVM or >even convince people that the answer to question #1 should be 'no' if we >have a code base that is mature, well supported, and has a healthy >userbase.The purpose of this thread is to notify the community that I am working on this so that there ends up only one proposal and we don't waste effort. I'll definitely polish and test thoroughly once I get intrinsics and the rest of the tablegen stuff done. I hope to get a large user base but this is sort of chicken and egg. Keeping it separate is not a viable longterm solution and is the primary reason why I want to upstream it eventually.>You don't need to register intrinsics to be able to use them, and >it's also possible to register them without a backend, but this has not >been done before."Hasn't been bone" in an encouraging way, or not? And you base them on what, the collection of triples? (i.e. spirv: 32-bit OpenCL, spirv64: 64-bit OpenCL and possibly a Vulkan one if/when I have an idea on what to do with the (lack of) layout) Irrespectively my fork will stay a target for now.>You don't need a target for this. TargeLibraryInfo is constructed >based on the triple.Ah, OK, maybe it was that there wasn't a triple independent one available for the really simple operations when I was trying to enable it. Thanks, Nic -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170504/244c3f95/attachment.html>
I'd like to add my weight to this statement from Tom Stellard:> This goes back to something I mentioned in my original email, but > I really think the best thing to do for this project right now is to > keep it separate from LLVM, clean up the code, and try to get people > using it. It's going to be much easier to get this upstream in LLVM > or > even convince people that the answer to question #1 should be 'no' if > we > have a code base that is mature, well supported, and has a healthy > userbase.I think SPIR-V should go into tip LLVM eventually, I think it should not use SelectionDAG/GlobalISel because it doesn't make sense in my opinion. But the most important issue is getting LLVM based languages be able to target SPIR-V. I think it would be highly beneficial if any SPIR-V target was used to make external targets in LLVM/Clang work much more nicely. Being blunt - external targets in LLVM suck, you're forced to patch the source code. If a SPIR-V target could be used to: - Allow experimental targets to be discovered outwith the LLVM tree (EG. the target doesn't have to be in lib/Target) - Move target triple information into the lib/Target/* folders (or at least allow a target to register a triple and information about the target in the target folder) That'll get us a huge step towards being able to use LLVM with external targets, and also allow a bunch of developers to use a SPIR-V backend as if it was in tip LLVM, thus demonstrating its viability for upstream. Cheers, -Neil. On 2017-05-03 20:04, Tom Stellard via llvm-dev wrote:> On 05/03/2017 11:19 AM, Nicholas Wilson wrote: >>> Right, what I was trying to say is that there are more benefits from >> >>> having this not be a target than there is from having it be a target. >> >> Please enumerate them, I have seen none posted so far . The implied >> “it is what all the the other backends do” w.r.t ISel/MC is at >> best(worst?) an implementation detail, and I’m still not quite sure >> why Chandler was so adamant about that. He seemed to imply that >> generating straight from the IR (as opposed to post legalisation?) >> introduces a direct dependance in the IR that the rest of LLVM would >> then be required to not break? I agree that the SPIRV backend should >> be insulated from changes the IR, although I’m not sure how to achieve >> that property. I’m also not sure how much, if at all, it would be >> susceptible to that to begin with. Deletions of >> instructions/attributes would obviously cause breakage and additions >> may cause unhandled and/or invalid combinations. I still don’t get the >> severity if this though, insight appreciated. >> > > So there are really two questions here: > 1. Should targets be required to use SelectionDAG/GlobalISEL ? > 2. Should SPIR-V use SelectionDAG/GlobalISel? > > In my opinion, regardless of the answer to question #1, the answer > to question #2 is no, SPIR-V should not use SelectionDAG/GlobalISel. > > I touched on this before in previous emails, but the main problem is > that > SelectionDAG (and GlobalISel to a lesser extent) plus the whole > MachineInstr > layer is a much lower-level representation than SPIR-V, so you will > need to do a lot of extra work and/or modifications to existing > infrastructure in order to get a working target, and even then > you may be limited to emitting poor quality SPIR-V that other > backends will have a hard time optimizing. > > With all this work, what advantages are you getting? If the > only reason to do it this way is so you can use intrinsics, > or TargetLibraryInfo, or easier integration with other tools, > I think it would be better to try to save the effort and try > to solve those problems in some other way. > > LLVM IR -> SPIR-V directly will give you better code, lower compile > times. It will be more simple and easier to maintain, and you will > be able to re-use existing SPIR-V parsers/writers that exist > in SPIRV-Tools. > > This goes back to something I mentioned in my original email, but > I really think the best thing to do for this project right now is to > keep it separate from LLVM, clean up the code, and try to get people > using it. It's going to be much easier to get this upstream in LLVM > or > even convince people that the answer to question #1 should be 'no' if > we > have a code base that is mature, well supported, and has a healthy > userbase. > > >> >>> What you are proposing is a lot of work, and even if it does help to >>> avoid some duplicate work (which to be honest, I still don't quite >>> understand what duplication there would be if it weren't a target), >>> I don't think that is enough to justify the effort required. >> >> My point is that (modulo metadata, which I am still investigating >> better alternatives, and calling conventions) if SPIRV is a target, >> then a producer need not change their compilation pipeline /at all/ to >> target SPIRV. There should be no effort required, it would come as a >> property of being a target. I think we are confusing each other again >> :( >> >> Leaving that aside for a moment, there are a number of >> advantages/requirements that, correct me if I’m wrong, would be >> impossible without a proper target. >> >> * Most critically: Intrinsics. I am almost certain that you would not >> accept the current mangling hacks, and if I am to support windows >> neither can I. Any solution would therefore need to be able to >> register intrinsics and I believe this is impossible without a target >> (and even if it is, it makes less sense than a target that doesn’t use >> ISel/MC). Not being able to use intrinsics is a complete deal breaker. >> > > You don't need to register intrinsics to be able to use them, and > it's also possible to register them without a backend, but this has not > been done before. > >> *Basic optimisations (basic CSE,DCE,inlining): requires a >> TargetLibraryInfoImpl(?) which I believe requires a target. While not >> strictly necessary it would improve the readability of the resulting >> IR/SPIRV. All of the more complex optimisations would be done “post >> ingestion” of the SPIRV and with a different target triple so are >> unaffected by any decision made. See my reply to Hongbin for an >> approach. > > You don't need a target for this. TargeLibraryInfo is constructed > based on the triple. > > -Tom > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
On 5/3/2017 12:04 PM, Tom Stellard via llvm-dev wrote:> On 05/03/2017 11:19 AM, Nicholas Wilson wrote: >>> Right, what I was trying to say is that there are more benefits from >>> having this not be a target than there is from having it be a target. >> Please enumerate them, I have seen none posted so far . The implied “it is what all the the other backends do” w.r.t ISel/MC is at best(worst?) an implementation detail, and I’m still not quite sure why Chandler was so adamant about that. He seemed to imply that generating straight from the IR (as opposed to post legalisation?) introduces a direct dependance in the IR that the rest of LLVM would then be required to not break? I agree that the SPIRV backend should be insulated from changes the IR, although I’m not sure how to achieve that property. I’m also not sure how much, if at all, it would be susceptible to that to begin with. Deletions of instructions/attributes would obviously cause breakage and additions may cause unhandled and/or invalid combinations. I still don’t get the severity if this though, insight appreciated. >> > So there are really two questions here: > 1. Should targets be required to use SelectionDAG/GlobalISEL ? > 2. Should SPIR-V use SelectionDAG/GlobalISel? > > In my opinion, regardless of the answer to question #1, the answer > to question #2 is no, SPIR-V should not use SelectionDAG/GlobalISel. > > I touched on this before in previous emails, but the main problem is that > SelectionDAG (and GlobalISel to a lesser extent) plus the whole MachineInstr > layer is a much lower-level representation than SPIR-V, so you will > need to do a lot of extra work and/or modifications to existing > infrastructure in order to get a working target, and even then > you may be limited to emitting poor quality SPIR-V that other > backends will have a hard time optimizing. > > With all this work, what advantages are you getting? If the > only reason to do it this way is so you can use intrinsics, > or TargetLibraryInfo, or easier integration with other tools, > I think it would be better to try to save the effort and try > to solve those problems in some other way. > > LLVM IR -> SPIR-V directly will give you better code, lower compile > times. It will be more simple and easier to maintain, and you will > be able to re-use existing SPIR-V parsers/writers that exist > in SPIRV-Tools. > > This goes back to something I mentioned in my original email, but > I really think the best thing to do for this project right now is to > keep it separate from LLVM, clean up the code, and try to get people > using it. It's going to be much easier to get this upstream in LLVM or > even convince people that the answer to question #1 should be 'no' if we > have a code base that is mature, well supported, and has a healthy > userbase. >This is completely skipping over one very important step which is currently part of ISel: legalization. The LLVM optimizers expect backends to support arbitrary-width integers, arbitrary-width vectors, and target-independent intrinsics which are lowered by legalization. SPIR-V does not have native support for these, therefore you need the legalization framework. And the legalization framework in LLVM is fundamentally tied to ISel. -Eli -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
2017-05-03 21:04 GMT+02:00 Tom Stellard via llvm-dev <llvm-dev at lists.llvm.org>:> So there are really two questions here: > 1. Should targets be required to use SelectionDAG/GlobalISEL ?In the past we also had a C and an LLVM-IR builder-backend (also known as Cpp backend) which did not use instruction selection. That is, there are other uses cases for non-SelectionDAG/GlobalISEL backends. I have no use for the Cpp backend, but a revival of the C backend could be interesting. Michael