Skip to content

Simpify PassRunner.add() and automatically parallelize parallel functions #2242

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

Merged
merged 6 commits into from
Jul 19, 2019

Conversation

kripken
Copy link
Member

@kripken kripken commented Jul 19, 2019

Main change here is in pass.h, everything else is changes to work with the new API.

The add("name") remains as before, while the weird variadic add(..) which constructed the pass now just gets a std::unique_ptr of a pass. This also makes the memory management internally fully automatic. And it makes it trivial to parallelize WalkerPass::run on parallel passes.

As a benefit, this allows removing a lot of code since in many cases there is no need to create a new pass runner, and running a pass can be just a single line.

@kripken kripken requested a review from tlively July 19, 2019 18:44
@kripken
Copy link
Member Author

kripken commented Jul 19, 2019

Hmm... this makes everything super-slow. It's not the parallelism - removing the parallel part from the pass.h changes has no effect. No idea what's going on here...

@kripken
Copy link
Member Author

kripken commented Jul 19, 2019

Nevermind, there was something wrong with that checkout - even though cmake indicated it was a release build, and even though I rebuilt... it was as slow as a debug build. Building in a new directory works. This PR should be good to go.

Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

🎉

@kripken kripken merged commit 88ef839 into master Jul 19, 2019
@kripken kripken deleted the assert2 branch July 19, 2019 23:01
tlively added a commit that referenced this pull request Jul 21, 2019
#2242 had exposed the bug that the `Trapper` pass was defining `walkFunction` when it should have been defining `doWalkFunction`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants