[llvm-dev] [SPIR-V] SPIR-V in LLVM

[Tom Stellard via llvm-dev]

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