[PATCH] D71617: [WIP][NFC][Attributor] noalias attribute deduction fixme

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D71617#1814061 <https://reviews.llvm.org/D71617#1814061>, @pgode wrote:

> Thanks for reviewing further.
>
> - Removed the Dependence flag.
> - For the conditions to return 'true', in Pred: I observe that the correct condition should be below. This condition means that callsite_argument should not have nocapture and getCtxI() should be reachable from user. I think this, because for noalias propagation to happen w.r.t. (ii) condition, we should get true from checkForAllUses and Pred as well.
>
>   ``` // Doesn't have nocapture but reachable. if (!NoCaptureAA.isAssumedNoCapture() && ReachabilityAA.isAssumedReachable(UserI, getCtxI())) return true;
>
>   ```


I don't follow, but maybe I'm just to tired. Let me try to explain:
What we want is to argue that the use we look at can not introduce aliasing through "unrelated" pointers. We know this is the case if (1) the use is either a `nocapture` use, thus `nocapture` is assumed, *or* (2) it is a use from which the current context instruction is not reachable. 
Hence the condition would be:

  if (auto *CB = dyn_cast<CallBase>(UserI)) {
    ...
    if (NoCaptureAA.isAssumedNoCapture())
      return true;
  }
  if (!ReachabilityAA.isAssumedReachable(UserI, getCtxI())))
    return true;
  ``
  
  
  > - For the pthreads.ll test case, are we conservatively assuming that when we propagate 1st noalias attribute then there is potential of it getting
  > captured and thus we should not have noalias attribute propagation for other 3 cases ?  
  
  Yes, that is basically what I think as well.
  
  >   As per current conditions, as all users are reachable and the  [THREAD] parameter doesn't have nocapture, so noalias propagation will happen and the Pred handles 1 use at a time so not sure (and even with Dependence flag set to true), we can get only 1st user parameter to have noalias.  I apologize as I might be missing something basic.  
  
  I think the condition right now is the problem. What happens if you use my proposal above?


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

https://reviews.llvm.org/D71617