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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I think we make progress here. I haven't gone through all test changes because they might change again once you worked on the comments below.



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:848
+                       const AbstractAttribute &QueryingAA, const Value &V,
+                       const Instruction *ReachableFromI=nullptr);
 
----------------
Yes, this looks really useful!

Minor: Please clang format the code.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:2090
+    return true;
+  }
+
----------------
We should default to `false` in these methods as we don't know. I actually don't think we need them, see my comment at the use site.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2328
+      }
     }
 
----------------
Minor:
You can asssume V has pointer type so you don't need to check it.
To get the `CallSiteI` directly you can call `getCtxI()`.
I think you should even pass `CallSiteI->getNextNode()` to `checkForAllUses` because we don't want to see the uses in the call site.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:5071
+      }
+    }
+
----------------
I don't understand the code after the `if (ReachableFromI)` condition.
We should not care about the operation here and also not about nocapture because this is a generic utility function.

What you want to check if `ReachableFromI` is:

- Query the `AAReachability` for the function (you can do that before the worklist while loop actually).
- Ask `AAReachability` if `UserI` is reachable from `ReachableFromI`, if not skip the `UserI`.


================
Comment at: llvm/test/Transforms/Attributor/noalias.ll:229
+; CHECK-NEXT:    tail call void @use(i8* noalias [[A]])
+; CHECK-NEXT:    tail call void @use_nocapture(i8* noalias nocapture [[A]])
 ; CHECK-NEXT:    ret void
----------------
The last use should not be `noalias` because the on before is not `nocapure`!


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

https://reviews.llvm.org/D71617