[Profiler] Remove precursory compile-invoke in profiler run to make profiler robust to group2seq#2609
Open
ayakayorihiro wants to merge 14 commits intomainfrom
Open
[Profiler] Remove precursory compile-invoke in profiler run to make profiler robust to group2seq#2609ayakayorihiro wants to merge 14 commits intomainfrom
compile-invoke in profiler run to make profiler robust to group2seq#2609ayakayorihiro wants to merge 14 commits intomainfrom
Conversation
2b68f17 to
a5d4807
Compare
Contributor
|
This pull request has not seen activity in 14 days and is being marked as stale. If you're continuing work on this, please reply and state how to get this PR in a mergeable state or what issues it is blocked on. If the PR is not ready for review, please mark it as a draft. The PR will be closed in 7 days if there is no further activity. |
Contributor
Author
|
I'll take another close look at this PR next week; if it looks alright I will merge! |
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
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.
This PR makes the profiler's instrumentation approach robust to all intra-component optimization passes. (It's not yet robust to
inline- will make an issue about this.)For a long time, the profiler-specific compiler pipeline relied on having
compile-invokerun before profiler-specific passes (uniquefy-enablesandprofiler-instrumentation). Since we can only instrument within groups (except for continuous assignments), by havingcompile-invokerun at the very beginning, we were able to instrument wheninvokes were happening. However, this precursory run ofcompile-invokecaused cycle discrepancies withgroup2seq( #2470 ) and failure ininline(since structural enables of cells cannot be inlined).As always, any feedback will be very appreciated!
Approach
In order for us to instrument
invokes without having to runcompile-invoke, we decided to use invoke with comb groups. Specifically, for eachinvoke, we attach a special comb group inuniquefy-enables, which will be populated with probe wires inprofiler-instrumentation. For example, ininline.futilwe have:after running
uniquefy-enables, we get:and then after running
profiler-instrumentationwe get:Notes
This approach fully assumes that
uniquefy-enableswill always be followed byprofiler-instrumentation(andprofiler-instrumentationwill always be preceded byuniquefy-enables) sinceuniquefy-enablesadds an empty group. But I think this is ok for now since both passes will only be used under profiling!