[PATCH] D153305: [MemoryEffects][NFCI] Make the MemoryEffects class reusable

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/include/llvm/Support/ModRef.h:58
 
-/// Summary of how a function affects memory in the program.
-///
-/// Loads from constant globals are not considered memory accesses for this
-/// interface. Also, functions may freely modify stack space local to their
-/// invocation without having to report it through these interfaces.
-class MemoryEffects {
+class IRMemoryEffects {
 public:
----------------
nikic wrote:
> Why the enum wrapped in a class here?
Just to keep the `sth::Location` syntax around. A namespace would work, or I just rename the location into IRLocation and remove the class. No good reason not to do that.


================
Comment at: llvm/include/llvm/Support/ModRef.h:224
   bool onlyAccessesInaccessibleOrArgMem() const {
-    return isNoModRef(getModRef(Other));
+    return isNoModRef(getModRef(Location::Other));
   }
----------------
nikic wrote:
> This function assumes that there are only three locations. You need to make it `getWithoutLoc(Location::InaccessibleMem).getWithoutLoc(Location::ArgMem).doesNotAccessMemory()` to gracefully handle extra locations.
good catch


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153305/new/

https://reviews.llvm.org/D153305