[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

Note to self: don't forget to hit "submit". The comments below have been left unsubmitted for two weeks. Sorry about that.

The patch looks OK for the time being. That said, I do have concerns that we may be organically growing something that will be troublesome to deal with long-term.

TBH, I still can't quite make sense of where/how SPIR-V fits in the offloading nomenclature.

Right now we have multiple levels of offloading-related control points.

- offload targets, specified by --offload-arch. Determines the ISA of the GPU binary we produce.
- offload mechanism: OpenMP, CUDA runtime, HSA. Determines how we compile/pack/launch the GPU binaries.
- front-end: CUDA/HIP/ C/C++ w/ OpenMP.
- Driver: Determines compilation pipeline to glue everything together,

SPIR-V in these patches appears to be wearing multiple hats. 
It changes compilation pipeline, it changes offload mechanism and it changes offload targets. To further complicate things, it appears to be a derivative of the HIP compilation. I can't tell if it's an implementation detail at the moment, or whether it will become a more generic offload mechanism that would be expected to be used by other front- and back-ends. E.g. can we potentially compile CUDA code to target SPIR-V? Can OpenMP offload to SPIR-V?

So, the question is -- what's the right way to specify something like this in a consistent manner? 
`--offload` option proposed here does not seem to be a good fit. It was intended as a more flexible way to create a single `-cc1` sub-compilation and we're doing quite a bit more here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110622/new/

https://reviews.llvm.org/D110622