[PATCH] D135780: [IR] Switch everything to use memory attribute

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:7683
+        }
+        continue;
       }
----------------
jdoerfert wrote:
> This part (I think) broke a test, see below.
> 
> I'll assume the `|=` works as required here (though I find it counter-intuitive).
> The dropping of the attributes feels complex nevertheless. Can't we just drop the memory attribute all together?
> Can we verify the attribute is dropped from the IR?
Yeah, this code was incorrect. What I actually meant to do here is something like `ME |= MemoryEffects(MemoryEffects::Other, ME.getModRef())`, to indicate that this might potentially access a global as well.

In the interest of keeping this as close to NFC as possible, what I ended up doing is to preserve only the mod/ref information, and drop the location information, which should match what the code effectively did previously.


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

https://reviews.llvm.org/D135780