LLVM does not have valid assembly backends for atomicrmw on local memory. However, as this memory is thread local, we should be able to lower this to the relevant load/store.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I think the right place to deal with this is during the lowering. This pass just works around the actual problem.
llvm/lib/Target/NVPTX/NVPTXAtomicLower.cpp | ||
---|---|---|
80 | The description needs to be updated. |
No test.
llvm/lib/Target/NVPTX/NVPTXAtomicLower.cpp | ||
---|---|---|
71 | V is not a descriptive name. |
Few nits. LGTM overall.
llvm/include/llvm/Transforms/Scalar/LowerAtomic.h | ||
---|---|---|
17 | We could probably get by with a class AtomicRMWInst;. We don't need the whole header just for the pointer type. | |
32 | Nit: function names should be camel case, and start with a lower case letter (e.g. openFile() or isFoo())., according to LLVM style guide: https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | |
llvm/lib/Target/NVPTX/NVPTXAtomicLower.cpp | ||
64–66 | llvm::initializeNVPTXAtomicLowerPass(PassRegistry &); would do. | |
llvm/test/CodeGen/NVPTX/atomic-lower.ll | ||
7–12 | The file should be renamed to atomic-lower-local.ll and there should be a comment describing that the test ensures that we're able to lower atomics in local AS in principle and that in case of NVPTX we happen to lower them to regular load/stores. Without the explanation, the CHECK lines by themselves are somewhat puzzling as there is nothing about atomics in the IR they check for. |
We could probably get by with a class AtomicRMWInst;. We don't need the whole header just for the pointer type.