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

[JF Bastien via Phabricator via llvm-commits]

jfb added a comment.

Right now the control flow isn't clever, but I wonder if, as this analysis becomes more powerful, it'll have to act differently when `-fno-delete-null-pointer-checks` is specified? Is there a simple test that you can add to make sure null pointer checks don't cause false assumptions whenever this optimization becomes smarter?



================
Comment at: llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp:40
+    Value *PtrOp;
+    if (!match(&I, m_Load(m_Value(PtrOp))))
+      continue;
----------------
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.


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

https://reviews.llvm.org/D64258