Skip to content

gh-131798: Optimize _UNARY_NEGATIVE#135223

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
Merged
Fidget-Spinner merged 10 commits into from
Jun 23, 2025
Merged

gh-131798: Optimize _UNARY_NEGATIVE#135223

Fidget-Spinner merged 10 commits into from
Jun 23, 2025

Conversation

@noamcohen97
Copy link
Contributor

@noamcohen97noamcohen97 commented Jun 6, 2025•
edited by bedevere-app bot
Loading

@bedevere-appbedevere-appbot mentioned this pull request Jun 6, 2025
Open
Zheaoli
Zheaoli reviewed Jun 6, 2025
View reviewed changes
brandtbucher
brandtbucher approved these changes Jun 6, 2025
View reviewed changes
Copy link
Member

@brandtbucherbrandtbucher 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, just some suggested improvements. Thanks!

@noamcohen97
Copy link
ContributorAuthor

noamcohen97 commented Jun 7, 2025

Thanks for the review! @brandtbucher@Zheaoli

brandtbucher
brandtbucher approved these changes Jun 7, 2025
View reviewed changes
Copy link
Member

@brandtbucherbrandtbucher left a comment

Choose a reason for hiding this comment

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

Awesome! Can you just fix the merge conflict in the test file?

@noamcohen97
Copy link
ContributorAuthor

noamcohen97 commented Jun 9, 2025

Done!

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Jun 9, 2025

@noamcohen97 sorry it seems there's merge conflicts again from merging your other PR.

@noamcohen97
Copy link
ContributorAuthor

noamcohen97 commented Jun 9, 2025

@Fidget-Spinner I should have seen this coming 😄

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Jun 23, 2025

@noamcohen97 sorry I forgot to merge this in time and seems there's conflicts again 🤦 . Could you please merge in main again? I'll make sure to remember to merge it soon this time.

@Fidget-SpinnerFidget-Spinner self-assigned this Jun 23, 2025
@noamcohen97noamcohen97force-pushed the optimize-unary-negate branch from 72c83d8 to 517d7efCompareJune 23, 2025 18:47
@noamcohen97
Copy link
ContributorAuthor

noamcohen97 commented Jun 23, 2025

@Fidget-Spinner seems like _UNARY_NEGATIVE already got optimized in #135668 without considering float value and non-compact integer values.
I suggest we replace the current implementation with this one. WDYT?

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Jun 23, 2025

@noamcohen97 sounds good. Just make sure to use compact ints. Thanks!

Fidget-Spinner
Fidget-Spinner approved these changes Jun 23, 2025
View reviewed changes
Copy link
Member

@Fidget-SpinnerFidget-Spinner left a comment

Choose a reason for hiding this comment

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

Thanks!

@Fidget-SpinnerFidget-Spinner merged commit bda1218 into python:mainJun 23, 2025
55 checks passed
@noamcohen97noamcohen97 deleted the optimize-unary-negate branch June 23, 2025 19:42
AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025
@noamcohen97@AndPuQing
pythongh-131798: Optimize_UNARY_NEGATIVE(pythonGH-135223)
aed1ed0
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@noamcohen97@Pranjal095
pythongh-131798: Optimize_UNARY_NEGATIVE(pythonGH-135223)
3a28c51
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@noamcohen97@taegyunkim
pythongh-131798: Optimize_UNARY_NEGATIVE(pythonGH-135223)
f5a2a15
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@noamcohen97@Agent-Hellboy
pythongh-131798: Optimize_UNARY_NEGATIVE(pythonGH-135223)
9fc1522
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomasr8tomasr8tomasr8 approved these changes

@Fidget-SpinnerFidget-SpinnerFidget-Spinner approved these changes

@brandtbucherbrandtbucherbrandtbucher approved these changes

+1 more reviewer

@ZheaoliZheaoliZheaoli left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Fidget-SpinnerFidget-Spinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@noamcohen97@Fidget-Spinner@Zheaoli@tomasr8@brandtbucher