Skip to content

gh-131798: JIT: Narrow the return type of _CALL_LEN to int #132940

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

Merged
merged 4 commits into from
Apr 25, 2025

Conversation

diegorusso
Copy link
Contributor

@diegorusso diegorusso commented Apr 25, 2025

Reduce unnecessary guards whenever len() is called and used in arithmetic operations.

Reduce unnecessary guards whenever `len()` is called and used
in arithmetic operations.
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.

Thanks!

@Fidget-Spinner
Copy link
Member

Windows JIT builds might've been broken by #132852. That PR did not trigger JIT CI.

Comment on lines +1916 to +1920
a = [1, 2, 3, 4]
for _ in range(n):
_ = len(a) - 1

_, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if it's needed in this case, but for other tests there's normally a 'control' variable that checks that the result is still correct even with the optimization. Something like this:

Suggested change
a = [1, 2, 3, 4]
for _ in range(n):
_ = len(a) - 1
_, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
x = 0
a = [1, 2, 3, 4]
for _ in range(n):
_ = len(a) - 1
if _ == 3:
x += 1
_, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
self.assertEqual(res, TIER2_THRESHOLD)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK the problem is if I add this, _GUARD_TOS_INT and _GUARD_NOS_INT are appearing in the uops list.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we can't do it. x is from outside the loop (trace), so the first x += 1 has to have a guard somewhere.

Fidget-Spinner and others added 2 commits April 26, 2025 01:48
…e-131798.NpcKub.rst

Co-authored-by: Max Bernstein <tekknolagi@gmail.com>
@Fidget-Spinner Fidget-Spinner merged commit 4c20f46 into python:main Apr 25, 2025
58 checks passed
@diegorusso diegorusso deleted the call-len branch April 30, 2025 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants