[PATCH] D50391: [NVPTX] Select atomic loads and stores

[Jonas Hahnfeld via Phabricator via llvm-commits]

Hahnfeld added inline comments.


================
Comment at: lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp:853
 
+  // .acquire was only added with PTX ISA 6.0 / sm_70.
+  AtomicOrdering Ordering = LD->getOrdering();
----------------
tra wrote:
> I'd be more explicit -- `in order to lower atomic loads with stronger guarantees we would need to have to use .release/.acquire which are only available in ...`. 
> 
> Same for the tryStore() below.
> 
> Maybe add TODO to check if we *are* compiling for sm_70 and use ld/st with .release/.acquire qualifiers then.
I've updated the patch and added a note that we could also use `fence` instructions which were also added in `sm_70`. I'm not really sure if we could achieve the same effects using `membar`, but the PTX documentation doesn't mention them explicitly in the section about the "Memory Consistency Model"...


https://reviews.llvm.org/D50391