[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:235
   AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic;
+  bool IsNonTemporal = true;
 
----------------
If there are are no MMOs then the default ought to be false.


================
Comment at: lib/Target/AMDGPU/SIMemoryLegalizer.cpp:415
       return Changed;
     } else {
       reportUnknownSynchScope(MI);
----------------
Suggest removing "else" from these "if"s as each one handles a specific case and returns. So no need for "else if" since plain "if" will be sufficient. At this point want to give the unhandled error since the preceding "if"s should have handled all the cases, so putting it outside the else makes that clearer.

Same comment for other expand functions.


================
Comment at: lib/Target/AMDGPU/SIMemoryLegalizer.cpp:420
   }
 
+  if (MOI.isNonTemporal()) {
----------------
Suggest adding a comment along the lines of:

```
// Atomic instructions do not have the nontemporal attribute.
```

Same comment for other expand functions.


https://reviews.llvm.org/D36862