-
Notifications
You must be signed in to change notification settings - Fork 72
Turn inliner into a pass and use it in rewriter & optimizer #2149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the inlining and rewriting functionality by converting them into passes that integrate into the pass manager, improving modularity and consistency in the optimizer pipeline.
- Introduced RewritePass in rewriter to encapsulate pattern-based rewriting.
- Updated the optimizer to use a sequential pass manager that includes InlinePass, FoldConstantsPass, RewritePass, and unused removal passes.
- Refactored the inlining functionality by replacing _Inliner with InlinePass, which now returns a boolean flag indicating modifications.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
onnxscript/rewriter/init.py | Added RewritePass and updated rewrite function to use pass manager. |
onnxscript/optimizer/_optimizer.py | Reorganized optimizer passes to integrate new InlinePass and RewritePass. |
onnxscript/optimizer/_inliner.py | Converted _Inliner to InlinePass with a revised state reset and return value. |
❌ 35 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
print(f"Applied {count} of general pattern rewrite rules.") | ||
unused_remover = ir.passes.PassManager( | ||
|
||
rewrite_pass = ir.passes.PassManager( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is causing a regression when no rewrite-rules are specified. I think we should retain an if pattern_rewrite_rules
condition and avoid the RewritePass if no rules are specified.
In #2149 the logic for skipping rewrite when no rules are provided was removed. This PR adds the logic back and hardens input checks. Now if no rules are provided to `rewrite()`, it will only run cleanup passes.
…t#2149) Use passes in optimizer and rewriter. 1. By opting into using the pass infra early, we get the benefit of getting the additional features in pass infra w/o having to pay higher refactoring cost in the future. We will be able to add more sophisticated debug utilities/snapshot capabilities etc. to the passes. 2. Since we are offering the pass infra to users, we can start validating it internally by using it here. If order altering becomes a valid use case we can expect users may need that and we can create appropriate facilities to support the usage.
…t#2149) Use passes in optimizer and rewriter. 1. By opting into using the pass infra early, we get the benefit of getting the additional features in pass infra w/o having to pay higher refactoring cost in the future. We will be able to add more sophisticated debug utilities/snapshot capabilities etc. to the passes. 2. Since we are offering the pass infra to users, we can start validating it internally by using it here. If order altering becomes a valid use case we can expect users may need that and we can create appropriate facilities to support the usage.
…t#2149) Use passes in optimizer and rewriter. 1. By opting into using the pass infra early, we get the benefit of getting the additional features in pass infra w/o having to pay higher refactoring cost in the future. We will be able to add more sophisticated debug utilities/snapshot capabilities etc. to the passes. 2. Since we are offering the pass infra to users, we can start validating it internally by using it here. If order altering becomes a valid use case we can expect users may need that and we can create appropriate facilities to support the usage.
Use passes in optimizer and rewriter.