[PATCH] D64258: [InferFuncAttributes] extend 'dereferenceable' attribute based on loads

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp:40
+    Value *PtrOp;
+    if (!match(&I, m_Load(m_Value(PtrOp))))
+      continue;
----------------
jfb wrote:
> hfinkel wrote:
> > spatel wrote:
> > > hfinkel wrote:
> > > > Why not also stores (or AtomicCmpXchg or AtomicRMW)?
> > > Oversight on my part - tunnel vision based on the motivating cases. 
> > > 
> > > I've never done anything with the atomic opcodes, so I didn't remember them. For stores, how would dereferenceable aid optimization?
> > > Ok if we make this a TODO enhancement/follow-up?
> > > 
> > If you store to an address, then you know that it is dereferenceable. The point is not to aid the optimization of the store, but to aid the optimization of later loads. I'd like to see stores handled here - if we're going to find problems with this, that will make it more likely that we'll find them quickly.
> > 
> > Also, you must omit volatile loads and stores (IIRC, our semantics mean that volatile loads/stores won't imply dereferenceability for the non-volatile accesses).
> > 
> It does seem like you want to handle non-volatile atomic load / store, as well as cmpxchg and RMW.
Ok - one more TODO. :)


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

https://reviews.llvm.org/D64258