tra 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) ---------------- t4c1 wrote: > 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. Hmm. It should've worked. It would need the same explicit cast to a pointer in AS(3) as in your tests, but it would safeguard against attempts to pass it a generic pointer. E.g. https://godbolt.org/z/qE6oxzheM Explicit casting to AS(X) for AS-specific variants is annoying, but it's probably unavoidable at the moment regardless of whether we declare the pointer argument to be AS-specific or not. LLVM will not always be able to infer that a pointer is in particular AS. Using specific AS in the declaration has a minor benefit of safeguarding at compile time against unintentional use of generic pointers. Ideally we may want to convert generic variant of the builtin to AS-specific one, if LLVM does know the AS. We currently do this for loads/stores, but not for other instructions. ================ 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); + ---------------- t4c1 wrote: > 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. I guest the bast we can do here is to safeguard against unintentional use of generic pointer. There's not much we can do if someone explicitly casts a pointer to a wrong AS. I think declaring pointer arg to be in specific AS would be sufficient. 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
