Skip to content

GH-132508: Use tagged integers on the evaluation stack for the last instruction offset #132545

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.

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Apr 15, 2025

When reraising in a finally block, the exception needs to look as if it were raised from an earlier point in the code.
To do this we save the earlier instruction offset as an integer on the evaluation stack.
Currently, this requires boxing the integer, which can (extremely rarely) fail.
By using a tagged integer we can avoid that failure mode.

This is might seem like an elaborate fix for a very minor issue, and it is, but we will want tagged integers/pointers for many other things and this is a nice small step to that larger change.

See #132509

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

2 comments

@markshannon
Copy link
Member Author

As expected, performance is neutral.

Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

I believe the magic number needs to be updated (and everything regenerated too) since the exception_unwind label now pushes tagged ints.

@bedevere-app
Copy link

bedevere-app bot commented Apr 15, 2025

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@markshannon
Copy link
Member Author

I believe the magic number needs to be updated (and everything regenerated too) since the exception_unwind label now pushes tagged ints.

Nothing has changed in terms of stack effects. The only change is that we push a tagged int instead of a boxed one.

@markshannon
Copy link
Member Author

The failure on JIT/ARM64 look like a JIT bug. The assertion errors seem impossible and do not occur on any other platform.
Neither the ARM64 (JIT) interpreter or the other JIT builds show any of the failures.

@Fidget-Spinner
Copy link
Member

I think I've seen that JIT failure before on my old PR when I used a different tagging scheme.

@markshannon
Copy link
Member Author

The JIT ARM 64 windows failure is just a timeout for the int repr test that sometimes happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants