t4c1 added inline comments.

================
Comment at: clang/include/clang/Basic/BuiltinsNVPTX.def:1057
+
+BUILTIN(__nvvm_atom_xchg_global_i, "iiD*i", "n")
+TARGET_BUILTIN(__nvvm_atom_cta_xchg_global_i, "iiD*i", "n", SM_60)
----------------
tra wrote:
> We need to figure out how address-space-specific builtins are supposed to 
> work.
> Right now two competing approaches.
> 
> This patch declares builtins with generic pointer as an argument, but, 
> according to the test, expects to be used with the AS-specific pointer.
> It probably does not catch a wrong-AS pointer passed as an argument, either.
> It does happen to work, but I think it's mostly due to the fact that LLVM 
> intrinsics are overloaded and we happen to end up addrspacecast'ing  things 
> correctly if the builtin gets the right kind of pointer.
> 
> The other approach is to declare the pointer with the expected AS. E.g:
> > TARGET_BUILTIN(__nvvm_mbarrier_init_shared, "vWi*3i", "", AND(SM_80,PTX70))
> 
> IMO, this is the correct way to do it, but it is also rather inconvenient to 
> use from CUDA code as it operates on generic pointers.
> 
> @jdoerfert - WDYT?
>TARGET_BUILTIN(__nvvm_mbarrier_init_shared, "vWi*3i", "", AND(SM_80,PTX70))
>IMO, this is the correct way to do it, but it is also rather inconvenient to 
>use from CUDA code as it operates on generic pointers.

I tried doing this, however it is also completely unusable from OpenCL code 
(which is our use case). Trying to use it gives you errors about how casting 
pointers to different address spaces  - for example from local to AS3 is not 
allowed.


================
Comment at: clang/test/CodeGen/builtins-nvptx.c:557
+  // expected-error@+1 {{'__nvvm_atom_acquire_add_global_i' needs target 
feature sm_70}}
+  __nvvm_atom_acquire_add_global_i((__attribute__((address_space(1))) int 
*)ip, i);
+
----------------
tra wrote:
> What happens if I pass a wrong pointer kind? E.g. a generic or shared pointer?
It will silently accept it. I can look into how to output appropriate error 
message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112718

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to