Fixes #2358. Added to the reducer the ability to remove a function t… #2361
Conversation
… function that is not directly called. Factored out some code from the optimizer to help with this.
| std::vector<std::unique_ptr<ReductionOpportunity>> result; | ||
| // Consider each function. | ||
| for (auto& function : *context->module()) { | ||
| if (context->get_def_use_mgr()->NumUses(function.result_id()) > 0) { |
There was a problem hiding this comment.
I'm trying to think of cases where this might leave a function that can be removed. Recursion is not allowed, so that is not a problem. OpName will be removed by another reduction opportunity, so that will not be a problem.
The only thing that could be a problem are decorations. Right now I do not see any decorations that would cause you a problem. LinkageAttributes should force the function to be kept. FuncParamAttr is only used for kernels, which I presume you do not care about.
I think this is safe, and if we see a specific decoration in the future that is keeping it alive, then we should update at that time. Sorry, wrote this as I was thinking about it.
There was a problem hiding this comment.
The recursion restriction is actually added by Vulkan and is not core to SPIR-V. Not sure if this should only handle Vulkan cases.
There was a problem hiding this comment.
In general, where removal of Xs is blocked due to Ys not being removed, we should gradually file issues for, and then add features to, remove Ys. So while we definitely need to think more on removal of decorations, that should not affect this pass.
The issue of recursion is potentially interesting and I guess would be a problem if other passes are not able to break recursive cycles (by removing OpCall instructions).
I suggest we revisit that in future in response to modules that don't reduce well due to this, if we find them.
There was a problem hiding this comment.
I agree. Keep it simply for now, and see how all of the reduction opportunities work together. Improve as needed.
…es the function from the module.
…eferenced by a linkage decoration.
|
Clang format checks are to do with changes elsewhere. |
…hat is not directly called. Factored out some code from the optimizer to help with this.