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

[pankaj gode via Phabricator via llvm-commits]

pgode marked 3 inline comments as done and an inline comment as not done.
pgode added a comment.

Thanks for reviewing. I understand as this was during year end. 
I will make the changes and update the patch. I tried the changes locally and I think I need to further check why noalias is still propagated for other checks in pthread.ll .

Please help me with one clarification regarding checkForAllUses call.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2498
+            if (!ReachabilityAA.isAssumedReachable(UserI, getCtxI()))
+              return true;
+          }
----------------
jdoerfert wrote:
> Move this check before the `CallBase` conditional.
I agree. This doesn't depend on CallBase anyway.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2518
+      // UsePred checks: getCtxI() i.e. callSite, is reachable from V's user.
+      if (!A.checkForAllUses(UsePred, *this, V, getCtxI())) {
+        LLVM_DEBUG(
----------------
jdoerfert wrote:
> I don't think you should restrict it to uses reachable from `getCtxI` here. All uses are interesting and reachability is checked from the uses.
Please correct me if I am wrong, I understand that we don't need, where we were checking whether uses of V are reachable from getCtxI(). 
if (!A.checkForAllUses(UsePred, *this, V, getCtxI()))

Instead, we need the regular call which checks for all uses of V. 
if (!A.checkForAllUses(UsePred, *this, V)

And in that case, we won't need the ReachableFromI parameter to 'checkForAllUses'. And thus we can have the reachability check only in Pred instead of having one more check in checkForAllUses. 
Only problem with this would be w.r.t. setting AnyUnreachable, which records dependencies. 



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:5768
+        AnyUnreachable = true;
+      }
+    }
----------------
jdoerfert wrote:
> There is a `continue` missing in the above conditional.
I agree, for unreachable we don't need to check the predicate. 


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

https://reviews.llvm.org/D71617