-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Restructure function inliner #11731
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
Merged
Merged
Restructure function inliner #11731
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
6aa4119
Rewrite function inliner
gramalingam a0ea33d
Fix merge conflict
gramalingam 5679db2
Fix overlooked merge conflict
gramalingam be704d1
Add nested function call tests
gramalingam 4f08762
Add overload for Specialize
gramalingam 6fab021
Update C# generated protocol buffer code
garymm 369bc17
Update ONNX to 1.12
garymm 273aabb
dont change ONNX version in CI pipelines
garymm 53b84af
update onnx commit
garymm 5cc9ca5
install latest onnx package
garymm 3d496ea
latest onnx 1.12 commit
garymm fead0e0
DO NOT SUBMIT temporarily disabling opset 17 tests
garymm 7c97cf3
onnx 1.12.0 rc4
garymm 32d1e80
Merge branch 'onnx-1.12' into rama/fun-inliner
garymm 9e5af64
Pass symboltable to onnx shape inference
gramalingam f279243
Avoid renaming empty names
gramalingam 11e1d12
Merge with master
gramalingam 3d666d9
Enable sequence_map tests
gramalingam File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
copy probably is fine as there is no initializers in function proto. but do we really need a copy?
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.
Good question. I thought about it earlier. But decided this was fine for current use, because we end up changing the names/attributes when inlining. So, we end up with a copy anyway. (The current implementation reduces the number of intermediate-representations used during inlining from two to one. Reducing the one to zero is possible, but would complicate the code, so settled on this.)
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 see.