[PATCH] D134527: [FunctionAttrs] Infer precise FMRB

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:206
+    // Non-call instructions can access either only argument memory, or
+    // argument & other memory, but never inaccessible memory.
+    MRB |= FunctionModRefBehavior::argMemOnly(MR);
----------------
reames wrote:
> The claim that no instruction other than a call can access inaccessiblememory seems reasonable on the surface, but is a somewhat significant change.  The corner cases would be something like maybe an indirect br.  
> 
> If possible, could you separate this into it's own commit for risk reduction?  Having something easy to revert could be helpful.   
Interesting point. Thinking about this again, I think there are two cases here:

1. If we have a MemoryLocation, then only that location can be accessed and an IR location can't be inaccessible by definition.

2. If we don't have a MemoryLocation (but still access memory), then things are less clear cut. Checking the implementation, the (non-call) instructions for which this happens are fence, catchpad and catchret.

In the interest of being conservative, it probably makes sense to keep the inaccessiblemem access in the latter case. I'm not familiar enough with the semantics of these instructions to exclude this -- e.g. could a fence synchronize atomic operations on an inaccessible object?


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

https://reviews.llvm.org/D134527