[PATCH] D99135: [deref] Implement initial set of inference rules for deref-at-point

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/IR/Value.cpp:756
+    // on F's behalf either.
+    if (A->hasNoFreeAttr() && A->hasNoAliasAttr())
+      return false;
----------------
nlopes wrote:
> reames wrote:
> > nlopes wrote:
> > > I don't understand this argument of why hasNoSync isn't needed.
> > > All argument attributes are with respect to what the function does, not the environment. It would be impossible to infer attributes if the environment was considered.
> > > 
> > > My understanding is that readonly implies nofree. Can't see why nofree would have those special semantics
> > The focus here isn't on nofree implying nosync.  It's noalias which implies nosync.  In order to for there to be a written (freed) copy of the pointer accessible in the other thread, the noalias fact must be false.  If so, that's already UB.
> Ok, I confess I'm less familiar with the noalias attribute. Though memcpy's signature is something like:
>   declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg) #1
> 
> The caller of memcpy may have had the src/dst pointers escaped to other threads. Even then, both arguments are marked as noalias because the C spec says the arguments cannot overlap.
> This contradicts the global definition of noalias you are suggesting.
I'm also fairly unfamiliar with noalias.  I'm going off existing wording, and what the optimizer appears to actually do.

I don't think it actually is contradictory.  Your concern involves a *writeable* copy of the pointer.  The wording of noalias talks about *access*.  It doesn't appear to be UB to have a writeable copy of the pointer on another thread (or even within the callee).  It does appear to be UB to *actually write to it* during the execution of the callee.  

Note that this definition does make noalias argument attributes very hard to infer.  I'll note that we only have noalias return inference which might be exactly because of that.  :)  I'm not sure.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99135