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.
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
[Analysis] Add OpCount Analysis #7644
[Analysis] Add OpCount Analysis #7644
Changes from 5 commits
377ec7f
a8fcd67
8d90aa1
c6f3887
bb1b070
9cc78db
e479a9e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
opCounts
could be calculated fromoperandCounts
here, but it would require a lot of extra iterating overDenseMap
s so went for the faster option since we probably don't want this to be slower than necessary if it's running on huge blocks of IR.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.
If you plan to add a proper pass to print it as JSON, why do we even need a separate test pass? That proper pass could support two formats (a human-readable one and an easy-to-parse one) like the upstream variant.
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 point - I just put this in so we had something upstream quickly to start getting data while I did the JSON pass, but definitely agree it would make sense to have the JSON pass also do a human readable form.
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'll leave this format to you, particularly since you mentioned bridging this to JSON or something like that.
I tend to actually export this information directly as YAML (manually, not using the internal library). You may be able to go to JSON directly here if you want.
Regardless, I think the format should be less wordy eventually, e.g.:
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.
Yeah, I just put this in for testing so figured something readable in case of debugging failure wouldn't hurt. Happy to trim it down if preferred though.