Skip to content

gh-145866: Convert _CALL_INTRINSIC_2 to leave its inputs on the stack to be cleaned up by _POP_TOP - #146262

Merged
Fidget-Spinner merged 6 commits into
python:mainfrom
KevinH15291:gh-145866-call_intrinsic_2
Mar 26, 2026
Merged

gh-145866: Convert _CALL_INTRINSIC_2 to leave its inputs on the stack to be cleaned up by _POP_TOP#146262
Fidget-Spinner merged 6 commits into
python:mainfrom
KevinH15291:gh-145866-call_intrinsic_2

Conversation

@KevinH15291

@KevinH15291KevinH15291 commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

@python-cla-bot

python-cla-botBot commented Mar 21, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@KevinH15291

Copy link
Copy Markdown
ContributorAuthor

This is my first CPython PR and I'm not very experienced. This PR largely just copies the style of the other PRs in #145866 and #134584, mainly #145964.

I'm not sure if

self.assertLessEqual(count_ops(ex, "_POP_TOP"), 4)

in my test is completely correct, as well as my testfunc

deftestfunc(n):
x=0for_inrange(n):
deftest_testfunc[T](n):
passreturnx

I'm also not 100% sure if regenerated the opcode metadata stuff correctly. Otherwise I believe the PR should be good.

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, this looks really good, thanks for doing this!

Comment threadLib/test/test_capi/test_opt.py Outdated
uops = get_opnames(ex)

self.assertIn("_CALL_INTRINSIC_2", uops)
self.assertEqual(count_ops(ex, "_POP_TOP_NOP"), 2)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

GreaterEqual might be better. Also did you count how many POP_TOP_NOPs this produces? You can print the trace logs by compiling with the tier 2 interpreter (--enable-experimental-jit=interpreter) and passing the env var PYTHON_LLTRACE=2.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Well honestly, I got lucky; I doubled the numbers used in the test for _CALL_INTRINSIC_1. Later I did verify this, by uh, modifying the end of the test function to print uops, and looking at the printed output, as I was not aware of PYTHON_LLTRACE=2 yet.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok cool, thanks for verifying. We want to use GreaterEqual here in case we introduce more optimizations in the future, and we don't want to cause this to break.

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@Fidget-Spinner

Copy link
Copy Markdown
Member

@KevinH15291 sorry could you please pull in main and regenerate the files again? Thanks!

@KevinH15291KevinH15291 changed the title gh-145866: Eliminate redundant refcounting from _CALL_INTRINSIC_2gh-145866: Convert _CALL_INTRINSIC_2 to leave its inputs on the stack to be cleaned up by _POP_TOPMar 25, 2026
@Fidget-Spinner
Fidget-Spinner merged commit f5364ae into python:mainMar 26, 2026
74 checks passed
@Fidget-Spinner

Copy link
Copy Markdown
Member

@KevinH15291 thank you for your contribution to CPython, and welcome!

clin1234 pushed a commit to clin1234/cpython that referenced this pull request Apr 16, 2026
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@KevinH15291@Fidget-Spinner@savannahostrowski