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-132732: Clear errors in JIT optimizer on error by Fidget-Spinner · Pull Request #136048 · python/cpython

Conversation

@Fidget-Spinner

@Fidget-Spinner Fidget-Spinner commented Jun 27, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: Constant evaluate/propagate pure ops automatically #132732

Sorry, something went wrong.

Uh oh!

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

All reactions
@Fidget-Spinner
Clear errors in JIT optimizer
267ecf7
@Fidget-Spinner
Fidget-Spinner requested a review from tomasr8 as a code owner June 27, 2025 16:19
@bedevere-app bedevere-app Bot added the awaiting core review label Jun 27, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Jun 27, 2025
Constant evaluate/propagate pure ops automatically #132732
Closed
@Fidget-Spinner Fidget-Spinner added the skip news label Jun 27, 2025
@Fidget-Spinner Fidget-Spinner mentioned this pull request Jun 27, 2025
gh-132732: Treat bytes as constants in _Py_uop_sym_is_safe_const #136033
Closed
@Fidget-Spinner
Fidget-Spinner requested a review from brandtbucher June 27, 2025 16:20
Fidget-Spinner added 2 commits June 28, 2025 00:27
@Fidget-Spinner
remove ability to return error
34c3f2b
@Fidget-Spinner
add assert
4eb9781
@tomasr8

tomasr8 commented Jul 1, 2025

Copy link
Copy Markdown
Member

Asking just to educate myself: what kind of errors are we expecting inside the JIT and is it safe to always clear them?

All reactions

Sorry, something went wrong.

Uh oh!

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

@brandtbucher

brandtbucher commented Jul 1, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

In general, we want to shift as much computation as possible to optimization-time instead of runtime. Some classes of errors, like MemoryError, are unavoidable. In these cases, we can just ignore them and trust that they'll either resolve themselves or surface in normal user code soon (it's a bit confusing if we raise an error like this in seemingly "random" places where JIT compilation may kick in).

The other class of errors are ones that are easier to ask forgiveness than permission for. For example, constant-evaluating stuff like a / b, a % b, a >> b, a << b, a[b], and so on, where a and b are known "safe" constants in the JIT. It's way easier to just try the operation and assume it will succeed, and just walk back any errors when it doesn't (in this latter case, the error will just happen at runtime when the operation is evaluated "for real").

tomasr8 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.

brandtbucher
brandtbucher reviewed Jul 1, 2025
View reviewed changes
Comment thread Python/optimizer_analysis.c Outdated
Comment on lines +562 to +564
if (PyErr_Occurred()) {
PyErr_Clear();
}

@brandtbucher brandtbucher Jul 1, 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.

Shouldn't this be:

Suggested change
if (PyErr_Occurred()) {
PyErr_Clear();
}
assert(PyErr_Occurred());
PyErr_Clear();

...or are there cases where an error hasn't occurred, but we end up here?

Sorry, something went wrong.

Uh oh!

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

All reactions

@Fidget-Spinner Fidget-Spinner Jul 2, 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.

I think the current code goes to error on some cases where we run out of memory too, but no memoryerror is set.

Sorry, something went wrong.

Uh oh!

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

All reactions

@brandtbucher brandtbucher Jul 2, 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.

I don't think so, all uses of goto error in these files seem like they have an error set.

Sorry, something went wrong.

Uh oh!

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

All reactions
Fidget-Spinner added 2 commits July 3, 2025 19:00
@Fidget-Spinner
Update optimizer_analysis.c
412ec63
@Fidget-Spinner
Update optimizer_analysis.c
26e2978
@Fidget-Spinner

Fidget-Spinner commented Sep 1, 2025

Copy link
Copy Markdown
Member Author

@tomasr8 may I trouble you with a review please?

All reactions

Sorry, something went wrong.

Uh oh!

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

tomasr8
tomasr8 approved these changes Sep 1, 2025
View reviewed changes

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

Given Brandt's explanation, this looks good to me :) I just left one question/comment

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Python/optimizer_analysis.c Resolved

Uh oh!

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

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Sep 1, 2025
Fidget-Spinner added 2 commits September 2, 2025 20:40
@Fidget-Spinner
Update comment
87505e1
@Fidget-Spinner
Merge remote-tracking branch 'upstream/main' into clear-error-on-opti…
2f256d4
…mizer-failure
@Fidget-Spinner
Fidget-Spinner requested a review from markshannon as a code owner September 2, 2025 12:40
@Fidget-Spinner
Fidget-Spinner merged commit 46f823b into python:main Sep 15, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Sep 15, 2025
@Fidget-Spinner
Fidget-Spinner deleted the clear-error-on-optimizer-failure branch September 15, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandtbucher brandtbucher brandtbucher left review comments
@tomasr8 tomasr8 tomasr8 approved these changes
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

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

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