[PATCH] D123531: [GlobalsModRef][FIX] Ensure we honor synchronizing effects of intrinsics

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

Sorry, that was a bit of a jumble of different ideas that I pressed into the form of a single message.  Let me try to do better.

Effects summarization attributes like `readnone` and so on have to be understood as encompassing the entire behavior of the call.  That of course includes any callbacks into visible code the call might make.  So the absence of `nocallback` doesn't weaken other attributes in any way.

I think the right way of thinking about synchronization, effects-wise, is that it is effectively a potential store to any memory that might be accessible by another thread.  Synchronization is therefore necessarily excluded by `readonly`, `argmemonly`, and so on.  We can usually ignore the possibility of stores happening concurrently on other threads, but synchronization forces us to honor them, and that is indistinguishable, in terms of effects, from the store happening directly as part of the call.

I haven't read much about `nocallback`, so I don't entirely understand what the optimization goal is.  If it's not implied by the existing effects attributes, then I guess we need to add it everywhere, and if that implies test churn, so be it.  But I want to make sure that we're not modeling something useless in the optimizer just because we can imagine code that it accurately describes; ultimately, the IR is a tool that exists to be of use to us as compiler developers, and perfect accuracy by itself is not a goal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123531