gh-145866: Eliminate redundant refcounting from _CALL_INTRINSIC_2#146262
gh-145866: Eliminate redundant refcounting from _CALL_INTRINSIC_2#146262KevinH15291 wants to merge 1 commit intopython:mainfrom
_CALL_INTRINSIC_2#146262Conversation
|
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 |
|
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 def testfunc(n):
x = 0
for _ in range(n):
def test_testfunc[T](n):
pass
return xI'm also not 100% sure if regenerated the opcode metadata stuff correctly. Otherwise I believe the PR should be good. |
Fidget-Spinner
left a comment
There was a problem hiding this comment.
Nice, this looks really good, thanks for doing this!
| uops = get_opnames(ex) | ||
|
|
||
| self.assertIn("_CALL_INTRINSIC_2", uops) | ||
| self.assertEqual(count_ops(ex, "_POP_TOP_NOP"), 2) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
_POP_TOP#145866