[PATCH] D119207: [CUDA][SPIRV] Assign global address space to CUDA kernel arguments

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10322
 ABIArgInfo SPIRVABIInfo::classifyKernelArgumentType(QualType Ty) const {
-  if (getContext().getLangOpts().HIP) {
+  if (getContext().getLangOpts().CUDAIsDevice) {
     // Coerce pointer arguments with default address space to CrossWorkGroup
----------------
shangwuyao wrote:
> jlebar wrote:
> > I am surprised by this change.  Is the language mode HIP only when compiling for device?  Or are you intentionally changing the behavior in HIP mode?
> > 
> > Same in SPIR.h
> We are targeting SPIRV so //I think// "compiling for device" is implied, I will let others comment on this to see if the assumption is correct. So this function can only be called when compiling for device, and won't be called when compiling for host. 
> 
> Also tried compiling for device and host separately to see where exactly does the code diverge (to make sure those two functions are not called when compiling for host):
> 1. This `classifyKernelArgumentType()` function is called from [[ https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGCall.cpp#L774-L777 | here ]], which is only enabled when the calling convention is `SPIR_KERNEL`. And when compiling for host, the calling convention is `C`.
> 
> 2. For the SPIR.h file, the `TargetInfo::adjust` function is called both when compiling for host and for device, see [[ https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/SPIR.h#L142-L157 | here ]], while the `setAddressSpaceMap` function is only called when compiling for device (SPIRV).
> 
> In conclusion, those two functions won't be reached when compiling for host.
LGTM. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119207