[PATCH] D135780: [IR] Switch everything to use memory attribute
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 25 02:05:52 PDT 2022
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
More information about the llvm-commits
mailing list