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: Split `_CHECK_AND_ALLOCATE_OBJECT` into smaller uops by Sacul0457 · Pull Request #148433 · python/cpython

Conversation

@Sacul0457

@Sacul0457 Sacul0457 commented Apr 12, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor
  • 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
Sacul0457 added 3 commits April 12, 2026 18:40
@Sacul0457
initial commit to split the uops
9c745a0
@Sacul0457
revert test
d976e47
@Sacul0457
remove empty line
c72c139
@Sacul0457
Sacul0457 requested review from Fidget-Spinner, markshannon, savannahostrowski and tomasr8 as code owners April 12, 2026 10:47
@bedevere-app bedevere-app Bot added the awaiting review label Apr 12, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Apr 12, 2026
Better uop coverage in the JIT optimizer #131798
Open
@Sacul0457
Sacul0457 marked this pull request as draft April 12, 2026 10:47
@bedevere-app bedevere-app Bot removed the awaiting review label Apr 12, 2026
@Sacul0457

Sacul0457 commented Apr 12, 2026

Copy link
Copy Markdown
Contributor Author

Hey @Fidget-Spinner, I'm a little lost here
From my understanding:

  1. In _CHECK_OBJECT if tp->tp_version_tag == type_version is true, we remove the guard.
  2. Do we set callable to a const here like it currently does?
  3. If 1. is false, we need to add a type watcher.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner
Fidget-Spinner reviewed Apr 12, 2026
View reviewed changes
Comment thread Python/optimizer_bytecodes.c Outdated
assert(init != NULL);
assert(PyFunction_Check(init));
callable = sym_new_const(ctx, init);
sym_set_const(callable, init);

@Fidget-Spinner Fidget-Spinner Apr 12, 2026

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.

  1. This should go into the new _ALLOCATE_OBJECT
  2. It should be callable = sym_new_const(ctx, init); not sym_set_const(callable, init);. There's a difference --- the first overrides the current callable with a new one, the second sets the current callable to a known constant. If you read the code in bytecodes.c, you'll see it's the first one.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sacul0457 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
cocolato
cocolato reviewed Apr 12, 2026
View reviewed changes
Comment thread Python/optimizer_bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/optimizer_bytecodes.c Outdated
PyType_Watch(TYPE_WATCHER_ID, (PyObject *)type);
_Py_BloomFilter_Add(dependencies, type);
}
}

@cocolato cocolato Apr 12, 2026 •
edited
Loading

Uh oh!

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

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.

Same to bytecodes.c:

Suggested change
}
}
self_or_null = sym_new_not_null(ctx);

Sorry, something went wrong.

Uh oh!

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

@Sacul0457 Sacul0457 Apr 12, 2026

Copy link
Copy Markdown
Contributor 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.

Wait sorry, shouldn't this be sym_set_non_null?

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner Fidget-Spinner Apr 12, 2026

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.

No. Please look at bytecodes.c. It's creating a new object, self. That's what _ALLOCATE_OBJECT does. So it should be a new non-null. This is same as the above where it grabs a new __init__ callable. In both of these cases, we do not set the existing symbol, but create a new one. This is a good learning point as it's tricky and requires observing bytecodes.c!

Sorry, something went wrong.

Uh oh!

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

@Sacul0457 Sacul0457 Apr 12, 2026

Copy link
Copy Markdown
Contributor 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.

Ah ok

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Sacul0457 @cocolato
Partially address reviews
7885033 
Co-authored-by: Hai Zhu <[email protected]>
@Fidget-Spinner

Fidget-Spinner commented Apr 12, 2026

Copy link
Copy Markdown
Member

@Sacul0457 I'm going to push to this PR, as I think it's a little tricky to get right. Please add a test after I push.
Sacul0457 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner
handle probable and non-probable cases
13e2608
@Fidget-Spinner

Fidget-Spinner commented Apr 12, 2026

Copy link
Copy Markdown
Member

@Sacul0457 please add the following tests:

  1. One test where there's a class outside the trace, and needs to be inferred. E.g.
class MyPoint:
    def __init__(self, x, y):
        return None

def testfunc(n):
    point_local = MyPoint
    for _ in range(n):
        p = point_local(1.0, 2.0)
        p = point_local(1.0, 2.0)
  1. One test where it's directly using the class, and that gets constant folded and removed. E.g.
class MyPoint:
    def __init__(self, x, y):
        return None

def testfunc(n):
    for _ in range(n):
        p = MyPoint(1.0, 2.0)
        p = MyPoint(1.0, 2.0)
Sacul0457 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner
lint
fc0c169
@Fidget-Spinner Fidget-Spinner added the skip news label Apr 12, 2026
@Sacul0457

Sacul0457 commented Apr 12, 2026

Copy link
Copy Markdown
Contributor Author

Sorry for the wait, I printed the uops trace for the second test and how can we tell if it is constant folded?

['_START_EXECUTOR', '_MAKE_WARM', '_SET_IP', '_CHECK_PERIODIC', '_CHECK_VALIDITY', '_ITER_CHECK_RANGE', '_GUARD_NOT_EXHAUSTED_RANGE', 
'_ITER_NEXT_RANGE', '_SET_IP', '_SWAP_FAST_1', '_SPILL_OR_RELOAD', '_POP_TOP', '_CHECK_VALIDITY', '_SET_IP', '_LOAD_DEREF', 
'_CHECK_VALIDITY', '_PUSH_NULL', '_LOAD_CONST_INLINE_BORROW', '_SPILL_OR_RELOAD', '_LOAD_CONST_INLINE_BORROW', '_SET_IP', 
'_SPILL_OR_RELOAD', '_CHECK_OBJECT', '_ALLOCATE_OBJECT', '_CREATE_INIT_FRAME', '_PUSH_FRAME', '_CHECK_VALIDITY', 
'_TIER2_RESUME_CHECK', '_LOAD_CONST_INLINE_BORROW', '_SET_IP', '_RETURN_VALUE', '_CHECK_VALIDITY', '_SET_IP', '_EXIT_INIT_CHECK', 
'_CHECK_VALIDITY', '_SET_IP', '_MAKE_HEAP_SAFE', '_RETURN_VALUE', '_GUARD_IP_RETURN_VALUE', '_GUARD_CODE_VERSION_RETURN_VALUE', 
'_CHECK_VALIDITY', '_SET_IP', '_SWAP_FAST_2', '_POP_TOP', '_CHECK_VALIDITY', '_SET_IP', '_LOAD_DEREF', '_CHECK_VALIDITY', 
'_PUSH_NULL', '_LOAD_CONST_INLINE_BORROW', '_SPILL_OR_RELOAD', '_LOAD_CONST_INLINE_BORROW', '_SET_IP', '_SPILL_OR_RELOAD', 
'_CHECK_OBJECT', '_ALLOCATE_OBJECT', '_CREATE_INIT_FRAME', '_PUSH_FRAME', '_CHECK_VALIDITY', '_TIER2_RESUME_CHECK', 
'_LOAD_CONST_INLINE_BORROW', '_SET_IP', '_RETURN_VALUE', '_CHECK_VALIDITY', '_SET_IP', '_EXIT_INIT_CHECK', '_CHECK_VALIDITY', 
'_SET_IP', '_MAKE_HEAP_SAFE', '_RETURN_VALUE', '_GUARD_IP_RETURN_VALUE', '_GUARD_CODE_VERSION_RETURN_VALUE', '_CHECK_VALIDITY', '_SET_IP', '_SWAP_FAST_2', '_POP_TOP', '_JUMP_TO_TOP', ...]
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner

Fidget-Spinner commented Apr 12, 2026

Copy link
Copy Markdown
Member

@Sacul0457 that's strange, seems like it's not being constant folded in the second case. Let me fix that.
All reactions

Sorry, something went wrong.

Uh oh!

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

Sacul0457 commented Apr 12, 2026

Copy link
Copy Markdown
Contributor Author

Oh alright. I'm probably going to bed now, please feel free to add the tests too as I don't want to hold up this PR being merged. Thank you!
Fidget-Spinner reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Sacul0457
Sacul0457 marked this pull request as ready for review April 12, 2026 17:42
@bedevere-app bedevere-app Bot added the awaiting review label Apr 12, 2026
@Fidget-Spinner

Fidget-Spinner commented Apr 12, 2026

Copy link
Copy Markdown
Member

Sorry Sacul, I forgot to mention that you can't nest MyPoint in the test, it needs to be a global to be promoted. My bad, and good night!
Sacul0457 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner
add tests
a459c25
Fidget-Spinner
Fidget-Spinner approved these changes Apr 12, 2026
View reviewed changes

@Fidget-Spinner Fidget-Spinner left a comment

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.

Thank you for splitting the ops in bytecodes.c Sacul.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sacul0457 reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Apr 12, 2026
@Fidget-Spinner
Fidget-Spinner merged commit 18d7d90 into python:main Apr 12, 2026
78 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 12, 2026
prakashsellathurai added a commit to prakashsellathurai/cpython that referenced this pull request Apr 13, 2026
@prakashsellathurai
Revert "pythongh-131798: Split _CHECK_AND_ALLOCATE_OBJECT into smal…
769e5da 
…ler uops (pythonGH-148433)"

This reverts commit 18d7d90.
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
@Sacul0457 @cocolato
@Fidget-Spinner @ljfp
pythongh-131798: Split _CHECK_AND_ALLOCATE_OBJECT into smaller uops (…
8aac827 
…pythonGH-148433)

Co-authored-by: Hai Zhu <[email protected]>
Co-authored-by: Ken Jin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cocolato cocolato cocolato left review comments
@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@tomasr8 tomasr8 Awaiting requested review from tomasr8 tomasr8 is a code owner
@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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

@Sacul0457 @Fidget-Spinner @cocolato
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.