[PATCH] D71617: [WIP][NFC][Attributor] noalias attribute deduction fixme
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 25 16:24:54 PST 2019
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
More information about the llvm-commits
mailing list