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.

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

Add support to PassManager drawer to display stages #8934

Closed
mtreinish opened this issue Oct 17, 2022 · 3 comments · Fixed by #9128
Closed

Add support to PassManager drawer to display stages #8934

mtreinish opened this issue Oct 17, 2022 · 3 comments · Fixed by #9128
Assignees
Labels
good first issue Good for newcomers type: feature request New feature or request

Comments

@mtreinish
Copy link
Member

What should we add?

In #6403 we added the concept of a staged passmanger where the different sections of a pass manager are organized into named stages. However, when calling the draw() method the additional metadata for the stages are missed. We should modify the drawer for the StagedPassManager class to annotate the output diagram with the stages. The idea I had in mind was putting each stage as an outer box around the passes.

@mtreinish mtreinish added type: feature request New feature or request good first issue Good for newcomers labels Oct 17, 2022
@javabster javabster moved this to Tagged but unassigned in Contributor Monitoring Oct 18, 2022
@psschwei
Copy link
Contributor

I'd like to work on this

@psschwei
Copy link
Contributor

psschwei commented Nov 9, 2022

Just a quick sanity check to make sure I'm not overcomplicating things:

StagedPassManager inherits the draw() method from PassManager. Which, if I'm understanding correctly, just iterates over the list of passes and draws the graph without considering any possible stage. Thus, to add an outer box around the passes for each stage, we'd thus need to implement a new draw() method for StagedPassManager that:

  • builds a subgraph for each stage
  • links those subgraphs together (i.e. connects the last node in one stage to the first node in the next)
  • returns the final graph

If there's something obvious I'm missing (and I very well may be), feel free to point me in another direction...

@mtreinish
Copy link
Member Author

Yeah, that's basically the design I had in mind when I opened the issue. I think that plan makes sense.

@javabster javabster moved this from Tagged but unassigned to Assigned in Contributor Monitoring Nov 17, 2022
@javabster javabster moved this from Assigned to PR open / Contributor working on it in Contributor Monitoring Nov 17, 2022
@mergify mergify bot closed this as completed in #9128 Feb 1, 2023
@github-project-automation github-project-automation bot moved this from PR open / Contributor working on it to Done in Contributor Monitoring Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: feature request New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants