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

Tue Dec 14 12:17:13 PST 2021

efriedma added inline comments.

================
Comment at: llvm/lib/Analysis/GlobalsModRef.cpp:957-958
+                Call->hasFnAttr(Attribute::NoSync) ? ModRefInfo::NoModRef
+                : Call->onlyReadsMemory()          ? ModRefInfo::Ref
+                                                   : ModRefInfo::ModRef;
+            Known = unionModRef(
----------------
tra wrote:
> 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. 
> 
readnone and readonly both imply nosync; we treat synchronization as a read and write to memory.

In terms of intrinsics, we currently don't consistently add nosync attributes.  If we mark an intrinsic IntrNoMem, that should imply nosync, even without IntrNoSync. We shouldn't be trying to infer nosync from readnone/readonly in GlobalsAA.  We should probably hack IntrinsicEmitter::EmitAttributes to make that work.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115302