This patch implement instruction reachability for AAFunctionReachability
attribute. It is used to tell if a certain instruction can reach a function
transitively.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
9755 | I think this is a problem if you have invokes. | |
9771 | don't ignore the return value | |
9779 | no braces. | |
llvm/unittests/Transforms/IPO/AttributorTest.cpp | ||
209 | can we have a negative instcanreach test? |
- Explicitly handle InvokeInst.
- Add a option to note use backwards reachability.
- Don't use backwards reachability for transative queries (fixes the fixme)
- Address misc review.
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
9669 | Is this the right way? I would have thought we ignore CB if CBInst is not reachable from Inst, not the other way around. We need a test I suppose. | |
9695–9696 | At least add a TODO. To fix it proper you can look at the two successors blocks and the first non-debug instruction in them. | |
9703–9705 | Why no range loop? |
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
9703–9705 | markReachable call below erases Fn from the set we are iterating right now. It would be invalid to increment the iterator if Fn was already deleted right ? I think we have to do that before. |
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
9703–9705 | I see. We have a special iterator + range loop for that, maybe you want to look at make_early_inc_range and it's uses. |
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
9669 | We are doing an early return if the CB is not assumed reachable. If not we add it into the Result Vector. We later use the result of this function for QueryResolver::update or QueryResolver::isReachable. So the return true here doesn't mean anything. |
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
9669 | I was not clear. My concern is that we check if CBInst may reach Inst while we want to know if Inst may reach CBInst, no? |
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
9711–9716 | ||
9791 | Reachable is not updated if CanReachUnknownCallee is set true. We also never check for a cached value here. | |
9809 | One of these caused a copy and only the copy QuerySet got updated causing it never to find a fixpoint. | |
9826 | One of these caused a copy and only the copy QuerySet got updated causing it never to find a fixpoint. | |
9836 | One of these caused a copy and only the copy QuerySet got updated causing it never to find a fixpoint. |
Testing it more I see this crashing and the backwards stuff not working properly. Probably easiest for me to put a fix up here.
Spell inst out in the function name, pass function as reference, (or is a nullptr ok?)