bader added a comment. In D110622#3174113 <https://reviews.llvm.org/D110622#3174113>, @tra wrote:
> 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. From my POV, SPIR-V is "the ISA of GPU binary we produce" and we might need some changes at offloading-related control points: - offload mechanism: none of listed "offload mechanisms" (i.e. OpenMP, CUDA runtime, HSA) is able to handle SPIR-V natively. On the other hand, I'm not sure if there is a need in additional changes for all "offloading mechanisms". E.g. Intel's compiler implements OpenMP-offload to SPIR-V target using OpenMP runtime plug-in lowering OpenMP runtime calls to OpenCL/Level Zero. OpenCL and Level Zero <https://spec.oneapi.io/level-zero/latest/index.html> runtimes are able to compile and launch SPIR-V binaries. - front-end: if we compare SPIR to other ISAs, they change compilation pipeline as well (e.g. add new built-ins to expose ISA, add CodeGen library changes to emit ISA specific metadata, etc.). AMDGPU ISA <https://llvm.org/docs/AMDGPUUsage.html#llvm> or NVIDIA <https://docs.nvidia.com/cuda/nvvm-ir-spec/index.html> GPU <https://llvm.org/docs/NVPTXUsage.html> ISA changed front-end/compilation pipeline as well. Do you refer to some other non-ISA specific changes? BTW, shameless plug, the patch adding built-in functions and types for SPIR-V ISA is under review here - https://reviews.llvm.org/D108034. - Driver: front-end compiler doesn't support SPIR-V format yet (i.e. SPIR-V requires special encoding different from LLVM bitcode) , so Driver hooks up LLVM->SPIR-V translator tool to produce SPIR-V binary. > 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? Intel's compiler compiles OpenMP offload and SYCL to SPIR-V, so we definitely would like to target SPIR-V using other front-ends. > 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. Does `--offload-arch=spirv*` fit better here? If I understand the goal of this patch correctly, it tries to provide controls for changing offload target for HIP application from default (AMDGCN) to SPIR-V. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110622/new/ https://reviews.llvm.org/D110622 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
