[PATCH] D115302: GlobalsModRef should treat functions w/o nosync conservatively.

Mon Dec 13 15:30:03 PST 2021

tra added a reviewer: asbirlea.
tra added a subscriber: asbirlea.
tra added a comment.

Added @asbirlea as a reviewer for the GlobalsModRef tests.

In D115302#3184025 <https://reviews.llvm.org/D115302#3184025>, @tra wrote:

> It might be easier to send the patch and let interested parties review the changes. I'll do it shortly.

On a second thought, I just don't know enough to convert all targets to use `DefaultIntrinsic` correctly.

> Let's prioritize correctness and require nosync, tell people on the list to update their def files already.

SGTM.

================
Comment at: llvm/lib/Analysis/GlobalsModRef.cpp:957-958
+                Call->hasFnAttr(Attribute::NoSync) ? ModRefInfo::NoModRef
+                : Call->onlyReadsMemory()          ? ModRefInfo::Ref
+                                                   : ModRefInfo::ModRef;
+            Known = unionModRef(
----------------
I'm not quite sure whether this is completely correct.  

What if it's an intrinsic w/o `IntrNoSync` but with `IntrNoMem` property, which technically also indicates no side effects. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115302