Details
- Reviewers
jholewinski bader tra
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
| Time | Test | |
|---|---|---|
| 70 ms | x64 debian > LLVM.Bindings/Go::go.test |
Event Timeline
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 | We need to figure out how address-space-specific builtins are supposed to work. This patch declares builtins with generic pointer as an argument, but, according to the test, expects to be used with the AS-specific pointer. The other approach is to declare the pointer with the expected AS. E.g:
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? | |
| clang/test/CodeGen/builtins-nvptx.c | ||
| 557 | What happens if I pass a wrong pointer kind? E.g. a generic or shared pointer? | |
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 |
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. | |
| clang/test/CodeGen/builtins-nvptx.c | ||
| 557 | It will silently accept it. I can look into how to output appropriate error message. | |
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 | 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. 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. | |
| clang/test/CodeGen/builtins-nvptx.c | ||
| 557 | I guest the bast we can do here is to safeguard against unintentional use of generic pointer. I think declaring pointer arg to be in specific AS would be sufficient. | |
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 | Well, it does not work. See: https://godbolt.org/z/vM6bKncc4. Declaring the pointer to be in generic AS is the only way I got it to work. If you know a way to call a builtin declared with AS numbers in OpenCL, I am happy to do that instead. | |
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 | Could you help me understand how different address spaces are handled by OpenCL in clang and LLVM? AFAICT what happens is:
In other words, having specific-AS arguemt works as intended, we just have a mismatch between AS number used by OpenCL and AS number used by NVPTX and we're not allowed to cast between two specific AS. If that's indeed the case, then what we appear to be missing is the mapping from OpenCL AS to the target-specific AS, which should, ideally, be done by clang itself. So, if NVPTX-specific builtin operating on shared pointer is called with a pointer in OpenCL's equivalent of CUDA's __shared__, it should be mapped to AS(3). | |
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 |
clang makes a strong distinction between language and target address spaces since ~3.6 (was more loose before). Each target in clang defines a map between language and target address space (e.g. https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/SPIR.h#L25). The map is used in clang codegen to lower __local to the right target address space.
Same code but targeting SPIR: https://godbolt.org/z/4E3brzodq (ARM maps all languages address spaces to 0)
OpenCL languages do map to nvptx target address space, it is just form clang's semantic a language asp is strongly distinct from target ones (lang asp simply isn't understood as a number). So you can't mix and match without an explicit cast, even if the lang asp eventually lowers to that target. What is missing is the ability for the target builtin to accept a language asp if it lowers to the target asp. | |
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 |
Agreed. I think this should be a sensible thing to support. | |
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 | Ok, I will look into that, but it might take some time. | |
| clang/include/clang/Basic/BuiltinsNVPTX.def | ||
|---|---|---|
| 1057 | Thank you for looking into this. These patches will help a lot to fill in the gap we currently have in support for atomics on GPU. | |
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:
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?