[PATCH] D36862: AMDGPU: Handle non-temporal loads and stores

[Tony Tye via Phabricator via llvm-commits]

t-tye added inline comments.


================
Comment at: lib/Target/AMDGPU/SIMemoryLegalizer.cpp:336-337
+  if (MOI.IsNonTemporal) {
+    // FIXME: handle non-temporal atomic loads?
+    assert(!MOI.IsAtomic);
+
----------------
b-sumner wrote:
> t-tye wrote:
> > b-sumner wrote:
> > > t-tye wrote:
> > > > According to http://llvm.org/docs/LangRef.html#load-instruction :
> > > > ```
> > > > !nontemporal does not have any defined semantics for atomic loads.
> > > > ```
> > > > 
> > > > My reading of this is that it is allowed to have nontemporal on atomic loads, but the meaning of it is not defined by LLVM. If so an assert is not correct here.
> > > > 
> > > > My suggestion is that AMDGPU target treats it the same as non-atomic.
> > > I'd prefer ignoring the non-temporal in the case of atomics.  Note that one would have to resort to coding in IR to even get it applied; no languages have a means to request non-temporal atomic operations.
> > So another patch should update the LLVM IR documentation. The syntax for load/store atomic already does not list nontemprarl as an allowed attribute, but the text mentions it. And also add a validator test to enforce that non-temporal is not allowed on load/store/rmw atomic.
> Is a validator needed, or can we just update the documentation to state that any nontemporal on atomic operations is ignored?
Currently the documentation gives the syntax for atomic instructions as not allowing the nontemporal attribute. So would seem that the validator should enforce that syntax, or the syntax should be changed.


https://reviews.llvm.org/D36862