Do not check for shadowing repeatedly in inlining passes. #4396
Conversation
This file contains 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
kfcripps
force-pushed
the
4298-upstream
branch
from
c26bbd7 to
2ee93c5
Compare
fruffy
added
the
core
fruffy
reviewed
Feb 7, 2024
| @@ -51,9 +51,7 @@ class ActionsInliner : public AbstractInliner<ActionsInlineList, AInlineWorkList | |||
| std::map<const IR::MethodCallStatement *, const IR::P4Action *> *replMap; | |||
|
|
|||
| public: | |||
| explicit ActionsInliner(bool isv1) : refMap(new P4::ReferenceMap()), replMap(nullptr) { | |||
| refMap->setIsV1(isv1); | |||
There was a problem hiding this comment.
Any reason why you are removing the isv1 check here and in other places? I think we should deprecated that strange feature eventually but it seems like you may change some semantics here.
There was a problem hiding this comment.
I think this is because the ReferenceMap being passed in already has the isv1 flag set appropriately.
There was a problem hiding this comment.
Yes, that's correct.
ChrisDodd
approved these changes
Feb 11, 2024
kfcripps
added
the
compiler-performance
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We found that checking shadowing in
ResolveReferenceswas a big bottleneck for one of our apps, and it is done each time an inlining pass is repeated. We turned this off and added a newResolveReferencespass to check shadowing after all inlining is completed. This greatly helped to speed up these passes, which were taking the longest.Compilation times for one of our apps without these changes:
and with them: