Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
GH-130415: Optimize JIT path for _TO_BOOL_INT branching#130477
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ghost commentedFeb 22, 2025 • edited by ghost
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by ghost
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Lib/test/test_capi/test_opt.py Outdated
| trace = [] | ||
| for i in range(n): | ||
| # f is always (int) 0, but we can only prove that it's a integer: | ||
| f = i - i # at this point python knows f is an int, but doesnt know that it is 0 (we know it is 0 though) |
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.
Can you renamef in this function tozero?
Lib/test/test_capi/test_opt.py Outdated
| # def test_narrow_type_to_constant_bool_true(self): | ||
| # def f(n): | ||
| # trace = [] | ||
| # for i in range(n): | ||
| # # f is always True, but we can only prove that it's a bool: | ||
| # f = i != TIER2_THRESHOLD | ||
| # trace.append("A") | ||
| # if f: # Kept. | ||
| # trace.append("B") | ||
| # if not f: # Removed! | ||
| # trace.append("X") | ||
| # trace.append("C") | ||
| # if f: # Removed! | ||
| # trace.append("D") | ||
| # trace.append("E") | ||
| # trace.append("F") | ||
| # if not f: # Removed! | ||
| # trace.append("X") | ||
| # trace.append("G") | ||
| # return trace | ||
| # trace, ex = self._run_with_optimizer(f, TIER2_THRESHOLD) | ||
| # self.assertEqual(trace, list("ABCDEFG") * TIER2_THRESHOLD) | ||
| # self.assertIsNotNone(ex) | ||
| # uops = get_opnames(ex) | ||
| # # Only one guard remains: | ||
| # self.assertEqual(uops.count("_GUARD_IS_FALSE_POP"), 0) | ||
| # self.assertEqual(uops.count("_GUARD_IS_TRUE_POP"), 1) | ||
| # # But all of the appends we care about are still there: | ||
| # self.assertEqual(uops.count("_CALL_LIST_APPEND"), len("ABCDEFG")) |
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.
Can you remove this commented test?
Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Python/optimizer_bytecodes.c Outdated
| // else if (next_opcode == _GUARD_IS_TRUE_POP) { | ||
| // sym_set_const(value, Py_True); | ||
| // res = sym_new_type(ctx, &PyBool_Type); | ||
| // } |
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.
| // else if (next_opcode == _GUARD_IS_TRUE_POP) { | |
| // sym_set_const(value, Py_True); | |
| // res = sym_new_type(ctx, &PyBool_Type); | |
| // } |
Python/optimizer_bytecodes.c Outdated
| // removed and the narrowed value to be invalid: | ||
| if (next_opcode == _GUARD_IS_FALSE_POP) { | ||
| sym_set_const(value, Py_GetConstant(Py_CONSTANT_ZERO)); | ||
| res = sym_new_type(ctx, &PyLong_Type); |
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.
This is always a boolean, since it's the output of the_TO_BOOL op.
| res=sym_new_type(ctx,&PyLong_Type); | |
| res=sym_new_type(ctx,&PyBool_Type); |
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
…e-130415.ibOV6B.rstCo-authored-by: Brandt Bucher <brandtbucher@gmail.com>
Uh oh!
There was an error while loading.Please reload this page.
Thanks for the contribution. Unfortunately this has a critical flaw. |
Closing in favor of#130477. |
Uh oh!
There was an error while loading.Please reload this page.
@brandtbucher
Thanks for the help.