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-130415: Narrowing to constants in branches involving `is` comparisons with a constant by reidenong · Pull Request #143895 · python/cpython

Conversation

@reidenong

@reidenong reidenong commented Jan 15, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

This PR does constant narrowing in the JIT optimizer for branches with _IS_OP comparisons with the constant singletons True, False, None.

It introduces a new optimizer symbol, predicate, which describes a relationship between two objects. We create a new optimizer symbol upon encountering a comparison with a constant, and when we have definitive information on a object after a branch is taken (or not taken) we narrow it to a constant. Ideally, this new symbol will be easily extendable for other ops like (==, !=, contains etc.).

Unit tests intend to show constant narrowing by performing narrowing, then constant folding with other ops and showing the proof of the latter.

All feedback is welcome, with emphasis on the overall approach, how this new symbol fits into the lattice, and naming.

Thanks in advance.

  • Issue: Better constant narrowing in the JIT optimizer #130415

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
cocolato reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
reidenong added 3 commits January 13, 2026 22:04
@reidenong
Add failing regression tests
181d096
@reidenong
Refine failing regression tests
e30a46b
@reidenong
Add new predicate symbol and IS_OP constant narrowing
eba715e
@reidenong
reidenong requested review from Fidget-Spinner, markshannon, savannahostrowski and tomasr8 as code owners January 15, 2026 22:58
@bedevere-app bedevere-app Bot mentioned this pull request Jan 15, 2026
Better constant narrowing in the JIT optimizer #130415
Closed
13 of 17 tasks
@bedevere-app bedevere-app Bot added the awaiting review label Jan 15, 2026
@Fidget-Spinner Fidget-Spinner added the skip news label Jan 15, 2026
@Fidget-Spinner Fidget-Spinner self-assigned this Jan 15, 2026
@Fidget-Spinner

Fidget-Spinner commented Jan 15, 2026

Copy link
Copy Markdown
Member

This is pretty cool! Thanks for doing this, I'll review it soon.
reidenong 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.
markshannon
markshannon requested changes Jan 16, 2026
View reviewed changes

@markshannon markshannon 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.

I like the idea, it provides some more reasoning about values and fits nicely into the existing framework.

Thanks for doing this.

I would recommend changing the construction of predicates to be more relaxed and only check for constants when the predicate is resolved. It would be more powerful, easier to reason about and maybe faster.

As an example:

p0 = a is b
p1 = b is None
exit_if(not p1)
exit_if(not p0)
# a must be None

By restricting p0 to cases where a is a const or b is a const, we cannot infer that a is None after the second exit.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_optimizer.h Outdated
extern JitOptRef _Py_uop_sym_new_compact_int(JitOptContext *ctx);
extern void _Py_uop_sym_set_compact_int(JitOptContext *ctx, JitOptRef sym);
extern JitOptRef _Py_uop_sym_new_predicate(JitOptContext *ctx, JitOptRef subject_ref, JitOptRef constant_ref, JitOptPredicateKind kind, bool invert);
extern bool _Py_uop_sym_is_known_singleton(JitOptContext *ctx, JitOptRef sym);

@markshannon markshannon Jan 16, 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.

It doesn't matters whether something is a singleton or not, so you can drop this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Include/internal/pycore_optimizer_types.h Outdated

typedef enum {
JIT_PRED_IS,
// JIT_PRED_EQ,

@markshannon markshannon Jan 16, 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.

Either remove this, or add a TODO note that we'll be adding EQ, NE, etc.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Python/optimizer_bytecodes.c Outdated

op(_IS_OP, (left, right -- b, l, r)) {
b = sym_new_type(ctx, &PyBool_Type);
bool invert = (oparg != 0);

@markshannon markshannon Jan 16, 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.

It would be both more powerful and result in cleaner code, if we left all analysis until the predicate is resolved.
We will often have more information then.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Python/optimizer_symbols.c Outdated
return;
case JIT_SYM_PREDICATE_TAG:
if (!PyBool_Check(const_val) ||
(_Py_uop_sym_is_const(ctx, ref) &&

@markshannon markshannon Jan 16, 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.

Drop these extra checks. I'm not sure they are necessary and may be creating a contradiction where none exists.

If const_val is a boolean, then we can resolve the predicate, sym_apply_predicate_narrowing(ctx, flag, as_bool(const_val))

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Python/optimizer_symbols.c Outdated
}

JitOptRef
_Py_uop_sym_new_predicate(JitOptContext *ctx, JitOptRef subject_ref, JitOptRef constant_ref, JitOptPredicateKind kind, bool invert)

@markshannon markshannon Jan 16, 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.

We don't need to restrict this to the case where one of the values is a constant. If a is b is true, then a and b refer to the same object regardless of whether one is constant.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting review labels Jan 16, 2026
@bedevere-app

bedevere-app Bot commented Jan 16, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong added 3 commits January 16, 2026 16:14
@reidenong
Remove comment
8834e12
@reidenong
Refactor predicate optimizer symbol
d828f8c 
- Delay analysis until predicate resolves
- Generalize predicate to lhs, rhs
- Expand to accept all safe constants (not just singletons)
@reidenong
Remove unnecessary checks in sym_set_const
24ac326
@reidenong
reidenong requested a review from markshannon January 16, 2026 20:52
@reidenong

reidenong commented Jan 16, 2026

Copy link
Copy Markdown
Contributor Author

Hi @markshannon I think I've addressed the stated problems, a summary of changes is that

  • the predicate symbol now takes general lhs and rhs references
  • analysis is done when predicate resolves
  • all constants are accepted (not just singletons)

Additionally, you mentioned that for sym_set_const, we can resolve the predicate with sym_apply_predicate_narrowing(ctx, flag, as_bool(const_val)). I may be mistaken, but I think that it is not necessary to place this within sym_set_const, because for each _GUARD_IS_*_POP we will apply predicate narrowing first before setting the predicate symbol to const. (Is there a case where we set the predicate symbol to const not from _GUARD_IS_*_POP?)

Another implementation I initially considered would be to move the sym_apply_predicate_narrowing call from _GUARD_IS_*_POP to within sym_set_const, but I thought that might introduce an additional layer of indirection and make the code harder to reason about. Let me know if this would be a preferred approach.

Thanks!
All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner commented Jan 16, 2026

Copy link
Copy Markdown
Member

We need a test in the optimizer_symbols.c file (at the bottom, the last function).

Also CI is failing, can you please investigate? Thanks!
reidenong 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.
@reidenong
Merge branch 'main' into narrowing-to-constants-in-branches-with-is
8c95c5f
@python-cla-bot

python-cla-bot Bot commented Jan 16, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner commented Jan 16, 2026

Copy link
Copy Markdown
Member

CI failing due to #142911
All reactions

Sorry, something went wrong.

Uh oh!

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

reidenong commented Jan 16, 2026

Copy link
Copy Markdown
Contributor Author

CI failing due to #142911

ok, will take a look and get back asap
All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner commented Jan 17, 2026

Copy link
Copy Markdown
Member

@reidenong you should be able to pull in main now and it will build.
reidenong 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.
reidenong and others added 2 commits January 17, 2026 21:20
@reidenong
Merge branch 'main' into narrowing-to-constants-in-branches-with-is
d0a3ef7
@reidenong
Add predicate symbol tests
33b8920
@reidenong

reidenong commented Jan 17, 2026

Copy link
Copy Markdown
Contributor Author

@Fidget-Spinner I have added the unit tests, ptal. thanks!

@reidenong you should be able to pull in main now and it will build.

btw it builds now but what was the issue? I'm a bit confused on what happened.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong added 2 commits January 19, 2026 20:08
@reidenong
Refine narrowing checks
fb0f8dc
@reidenong
Update symbol lattice diagram
4601243
markshannon
markshannon requested changes Jan 19, 2026
View reviewed changes

@markshannon markshannon 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.

This looks correct, and thanks for updating the ascii art.

I've a couple of suggestions that should help long term maintenance, otherwise LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_optimizer.h Outdated
extern bool _Py_uop_sym_is_compact_int(JitOptRef sym);
extern JitOptRef _Py_uop_sym_new_compact_int(JitOptContext *ctx);
extern void _Py_uop_sym_set_compact_int(JitOptContext *ctx, JitOptRef sym);
extern JitOptRef _Py_uop_sym_new_predicate(JitOptContext *ctx, JitOptRef lhs_ref, JitOptRef rhs_ref, JitOptPredicateKind kind, bool invert);

@markshannon markshannon Jan 19, 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.

Now that the predicate is symmetrical (meaning that we treat the lhs and rhs the same, not that it is commutative), I think we should drop the invert parameter and field and only use the kind, adding an IS_NOT kind.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Python/optimizer_symbols.c Outdated
bool lhs_is_const = _Py_uop_sym_is_safe_const(ctx, lhs_ref);
bool rhs_is_const = _Py_uop_sym_is_safe_const(ctx, rhs_ref);

if (pred.kind == JIT_PRED_IS && (lhs_is_const || rhs_is_const)) {

@markshannon markshannon Jan 19, 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.

Can you use a switch, that way the compiler will complain about missing cases as we add more kinds.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting change review labels Jan 19, 2026
@bedevere-app

bedevere-app Bot commented Jan 19, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong and others added 6 commits January 20, 2026 01:35
@reidenong
Change to switch in predicate narrowing
142eff4
@reidenong
Refactor invert in predicate symbol to kind
c6f4a6a
@reidenong
Remove whitespace
d0f799f
@reidenong
Merge branch 'main' into narrowing-to-constants-in-branches-with-is
d98015f
@reidenong
Fix typo
21997bf
@reidenong
Fix case default
bfaa65b
@reidenong

reidenong commented Jan 19, 2026

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting change review and removed awaiting changes labels Jan 19, 2026
@bedevere-app

bedevere-app Bot commented Jan 19, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app
bedevere-app Bot requested a review from markshannon January 19, 2026 18:37
@reidenong
Fix import styling
b6c4019
Fidget-Spinner
Fidget-Spinner approved these changes Jan 21, 2026
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting change review labels Jan 21, 2026
@Fidget-Spinner

Fidget-Spinner commented Jan 21, 2026

Copy link
Copy Markdown
Member

Please fix the merge conflicts, thank you!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
reidenong and others added 3 commits January 22, 2026 10:40
@reidenong
Merge branch 'main' into narrowing-to-constants-in-branches-with-is
d13b9a3
@reidenong
Regen cases post merge
23ae776
@reidenong
Fix whitespace
9e34678
markshannon
markshannon approved these changes Jan 22, 2026
View reviewed changes

@markshannon markshannon 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.

This looks good now. Thanks for doing this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon
markshannon merged commit 0b08438 into python:main Jan 22, 2026
63 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 22, 2026
thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request Feb 15, 2026
@reidenong @thunder-coding
pythongh-130415: Narrowing to constants in branches involving is co…
c173575 
…mparisons with a constant (pythonGH-143895)
@reidenong
reidenong deleted the narrowing-to-constants-in-branches-with-is branch September 2, 2026 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon approved these changes
@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes
@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

@Fidget-Spinner Fidget-Spinner

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

@reidenong @Fidget-Spinner @markshannon
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.