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 Instruction Namer pass to PassManager #981

Merged
merged 3 commits into from Sep 28, 2023
Merged

Conversation

tbennun
Copy link
Contributor

@tbennun tbennun commented Aug 15, 2023

This PR adds the built-in instruction namer pass to PassManager. The instnamer pass is very useful to work with the Python bindings, especially when value names are discarded during compilation and ValueRef.name is empty.

@tbennun
Copy link
Contributor Author

tbennun commented Aug 15, 2023

For some reason that is likely unrelated to the PR, rtd is failing to build

@esc esc added the needtriage Needs to be triaged further label Aug 15, 2023
@esc
Copy link
Member

esc commented Aug 15, 2023

@tbennun indeed, it seems like there is an issue with a missing configuration file. I will investigate 🕵️

Screen Shot 2023-08-15 at 10 11 20

@esc
Copy link
Member

esc commented Aug 15, 2023

According to:

https://blog.readthedocs.com/migrate-configuration-v2/

I would say this is part of:

Monday, August 14, 2023: Do a second brownout (temporarily enforce this deprecation) for 24 hours: 00:01 PST to 23:59 PST (midnight)

So probably OK to just wait and restart the build tomorrow.

I will open a ticket that this must be resolved before 25 September.

@esc
Copy link
Member

esc commented Aug 15, 2023

I will open a ticket that this must be resolved before 25 September.

#982

@esc esc added 3 - Ready for Review and removed needtriage Needs to be triaged further labels Aug 15, 2023
@esc
Copy link
Member

esc commented Aug 16, 2023

RTD is building again

Copy link
Member

@sklam sklam left a comment

Choose a reason for hiding this comment

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

Can you add a test to exercise the pass?

@tbennun
Copy link
Contributor Author

tbennun commented Aug 22, 2023

@sklam Added both to the populate passes test and to a specific behavior test

@sklam
Copy link
Member

sklam commented Aug 23, 2023

@tbennun, The test looks good and I have manually confirmed the behavior as well. One last thing, can you add docs for the new pass at docs/source/user-guide/binding/optimization-passes.rst? Thanks!

@tbennun
Copy link
Contributor Author

tbennun commented Aug 23, 2023

@sklam of course! I don't know how I missed that. Added.

@sklam
Copy link
Member

sklam commented Aug 30, 2023

Thank you for the contribution! The PR is marked Ready to Merge and its just pending our CI resources to free up since we are in the middle of a release.

@esc esc added this to the v0.42.0-rc1 milestone Sep 13, 2023
@esc
Copy link
Member

esc commented Sep 13, 2023

I've added this to the 0.42 milestone, such that it'll be considered for review.

@esc esc merged commit d5aad07 into numba:main Sep 28, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants