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

gh-115480: Type / constant propagation for float binary uops #115550

Merged

Conversation

lazorchakp
Copy link
Contributor

@lazorchakp lazorchakp commented Feb 16, 2024

@lazorchakp
Copy link
Contributor Author

lazorchakp commented Feb 16, 2024

@Fidget-Spinner - opening this as a draft because I still have a few questions and it's waiting on some code from #115507.

  • Currently, the main changes are essentially untested. None of the tests actually hit the if (is_const(left) && is_const(right)) case. I get the feeling that this is expected for now, but it's entirely possible that I've just written bad tests.
  • how much do we care about runtime performance of the tier 2 optimizer? Should I prefer safety or speed? For example: PyFloat_AsDouble vs PyFloat_AS_DOUBLE
    • related: I was wishing for something like _PyFloat_Add, but ended up performing each calculation manually; this feels unsafe

@lazorchakp
Copy link
Contributor Author

Also, I'll be on vacation until Tuesday and may be slow to respond. Please feel free to use / modify this code however you see fit if you're waiting on it

@Fidget-Spinner
Copy link
Member

Nice work! This is pretty close, I will just push some changes since you're on holiday. Some points for possible consideration are that the tests should test for the float guard being eliminated as well.

@Fidget-Spinner Fidget-Spinner marked this pull request as ready for review February 16, 2024 15:13
@Fidget-Spinner Fidget-Spinner merged commit 13addd2 into python:main Feb 16, 2024
51 checks passed
@lazorchakp lazorchakp deleted the float-binary-op-constant-propagation branch February 20, 2024 01:44
@lazorchakp
Copy link
Contributor Author

Thanks for finalizing and merging this 🎉

@mdboom mdboom mentioned this pull request Feb 28, 2024
32 tasks
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
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.

2 participants