Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Simplify annotation computations in RvsdgTreePrinter class #649

Merged
merged 3 commits into from
Oct 6, 2024
Merged

Simplify annotation computations in RvsdgTreePrinter class #649

merged 3 commits into from
Oct 6, 2024

Conversation

phate
Copy link
Owner

@phate phate commented Oct 6, 2024 •
edited
Loading

Use std::count_if() instead of explicit iteration to compute relevant annotations in RvsdgTreePrinter class.

@phate phate requested a review from caleridas October 6, 2024 10:39
@phate phate assigned phate and unassigned phate Oct 6, 2024
@phate phate requested a review from haved October 6, 2024 10:39
phate
phate commented Oct 6, 2024
View reviewed changes
Copy link
Owner Author

@phate phate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can not make use of the CountWhere() method for the structural inputs and outputs in the AnnotateNumMemoryStateInputsOutputs() method yet, as we do not have ranges for these yet.

@phate phate enabled auto-merge (squash) October 6, 2024 10:41
@caleridas
Copy link
Collaborator

Would it not make more sense to defer to the std::count_if / std::ranges::count_if stdlib facilities?

@phate phate changed the title Add CountWhere() method to iterator_range class Simplify counting code in RvsdgTreePrinter class Oct 6, 2024
@phate phate changed the title Simplify counting code in RvsdgTreePrinter class Simplify annotation computations in RvsdgTreePrinter class Oct 6, 2024
@phate
Copy link
Owner Author

phate commented Oct 6, 2024

@caleridas I was not aware of them. Changed the PR. Thanks, I learned something!

@haved haved disabled auto-merge October 6, 2024 12:48
haved
haved approved these changes Oct 6, 2024
View reviewed changes
Copy link
Collaborator

@haved haved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, nice use of the STL

@haved haved enabled auto-merge (squash) October 6, 2024 14:17
@haved haved merged commit ed49c50 into master Oct 6, 2024
11 checks passed
@haved haved deleted the CountWhere branch October 6, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haved haved haved approved these changes

@caleridas caleridas Awaiting requested review from caleridas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@phate @caleridas @haved