[Arc] Add SplitFuncsPass #7027
[Arc] Add SplitFuncsPass #7027
Conversation
| @@ -46,6 +46,7 @@ std::unique_ptr<mlir::Pass> createLowerVectorizationsPass( | |||
| std::unique_ptr<mlir::Pass> createMakeTablesPass(); | |||
| std::unique_ptr<mlir::Pass> createMuxToControlFlowPass(); | |||
| std::unique_ptr<mlir::Pass> createSimplifyVariadicOpsPass(); | |||
| std::unique_ptr<mlir::Pass> createSplitFuncsPass(unsigned splitBound = 20000); | |||
There was a problem hiding this comment.
I think you should be able to use the options struct that table-gen produces for you. That way you don't have to redefine the default split bound 🙂
| std::unique_ptr<mlir::Pass> createSplitFuncsPass(unsigned splitBound = 20000); | |
| std::unique_ptr<mlir::Pass> createSplitFuncsPass(const SplitFuncsOptions &options = {}); |
There was a problem hiding this comment.
Very nifty, the redefinition was bugging me! Thanks!
|
@fabianschuiki added an Arcilator option to run the pass - occurred to me while checking it works correctly that since the pass currently operates on |
|
I think running on |
|
@maerhart thanks for the great review! Think that should be everything addressed, let me know if I missed anything before I merge! |
| return funcOp.emitError("Cannot split functions into functions of size 0."); | ||
| if (funcOp.getBody().getBlocks().size() > 1) | ||
| return funcOp.emitError("Regions with multiple blocks are not supported."); | ||
| assert(splitBound != 0 && "Cannot split functions into functions of size 0"); |
There was a problem hiding this comment.
The if above makes this impossible to be triggered, so we could remove it.
There was a problem hiding this comment.
Oops, thought I'd dropped that, thanks for the catch!
| opBuilder.create<ReturnOp>(funcOp->getLoc(), outValues); | ||
| } | ||
| // Create and populate new FuncOps | ||
| for (long unsigned i = 0; i < blocks.size() - 1; i++) { |
There was a problem hiding this comment.
Nit: inconsistent post and pre-increments, LLVM generally prefers pre-increment.
|
|
||
| opBuilder.setInsertionPointToEnd(funcBlock); | ||
| for (auto [j, el] : llvm::enumerate(args)) | ||
| replaceAllUsesInRegionWith(el, newFunc.getArgument(j++), |
There was a problem hiding this comment.
| replaceAllUsesInRegionWith(el, newFunc.getArgument(j++), | |
| replaceAllUsesInRegionWith(el, newFunc.getArgument(j), |
|
Happy for me to merge @maerhart? |
Of course, thanks for all the improvements! |
As proposed in #6298, this PR adds a pass that splits up large functions into smaller functions to (eventually - I haven't included it in the Arcilator pipeline in this PR, as we'll want to actually split up the compilation units when using the pass, which is still on the todo list) work around parts of the pipeline that scale very badly with large funcs. This uses some liveness analysis bits so that (only) the values that are needed further up the call stack are passed up.