[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/Basic/Targets/AMDGPU.cpp:57
+    Constant, // sycl_constant
+    Private,  // sycl_private
     Generic,  // ptr32_sptr
----------------
I feel like we may be outgrowing these address-space map arrays.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:4596
+                                            IRFuncTy->getParamType(FirstIRArg));
+        }
+
----------------
This seems problematic; code like this shouldn't be necessary in every place that uses a pointer value.  The general rule needs to be that expression emission produces a value of its expected type, so the places that emit pointers need to be casting to the generic address space.  We can avoid that in some special expression emitters, like maybe EmitLValue, as long as the LValue records what happened and can apply the appropriate transform later.

Also, in IRGen we work as much as possible with AST address spaces rather than IR address spaces.  That is, code like this that's checking for address space mismatches needs to be comparing the source AST address space with the destination AST address space and calling a method on the TargetInfo when a mismatch is detected, rather than comparing IR address spaces and directly building an `addrspacecast`.  The idea is that in principle a target should be able to have two logical address spaces that are actually implemented in IR with the same underlying address space, with some arbitrary transform between them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89909