GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-131798: JIT: Narrow the return type of `_CONTAINS_OP_SET` to bool by tomasr8 · Pull Request #132057 · python/cpython

Conversation

@tomasr8

@tomasr8 tomasr8 commented Apr 3, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

For context: #130415 (comment)

I did run make regen-cases and made sure that the test failed before making the change in optimizer_bytecodes.c :)

  • Issue: Better uop coverage in the JIT optimizer #131798

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
tomasr8 added 2 commits April 3, 2025 20:08
@tomasr8
Allow the JIT to remove an extra _TO_BOOL_BOOL
c82f940
instruction after _CONTAINS_OP_SET by setting
the return type to bool.
@tomasr8
Add news entry
6928552
@tomasr8
tomasr8 requested a review from Fidget-Spinner as a code owner April 3, 2025 18:17
@bedevere-app bedevere-app Bot added the awaiting review label Apr 3, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 3, 2025
Better uop coverage in the JIT optimizer #131798
Open
tomasr8
tomasr8 commented Apr 3, 2025
View reviewed changes
Comment thread Lib/test/test_capi/test_opt.py
Comment on lines +1616 to +1622
x = 0
s = {1, 2, 3}
for _ in range(n):
a = 2
in_set = a in s
if in_set:
x += 1

@tomasr8 tomasr8 Apr 3, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We might be able to simplify this further, I wasn't sure how much the peepholer would optimize out if I used e.g. 2 in s directly.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@brandtbucher brandtbucher Apr 5, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

2 in s would have been fine, but this works well too!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tomasr8 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@tomasr8
Fix linter
52c1a54
@tomasr8
tomasr8 requested a review from brandtbucher April 3, 2025 21:10
@brandtbucher brandtbucher self-assigned this Apr 4, 2025
Fidget-Spinner
Fidget-Spinner approved these changes Apr 4, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Apr 4, 2025
brandtbucher
brandtbucher approved these changes Apr 5, 2025
View reviewed changes
@brandtbucher
brandtbucher merged commit 85bc489 into python:main Apr 5, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 5, 2025
@tomasr8
tomasr8 deleted the jit-contains-op-set-narrow branch April 6, 2025 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes
@brandtbucher brandtbucher brandtbucher approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@brandtbucher brandtbucher

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@tomasr8 @Fidget-Spinner @brandtbucher
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.