Repository navigation
GH-130415: Optimize JIT path for _TO_BOOL_INT branching - #130477
Klaus117 wants to merge 5 commits into
Conversation
Sorry, something went wrong.
|
All commit authors signed the Contributor License Agreement. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
| 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.
Can you rename f in this function to zero?
Sorry, something went wrong.
| # 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.
Can you remove this commented test?
Sorry, something went wrong.
| // 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.
| // else if (next_opcode == _GUARD_IS_TRUE_POP) { | |
| // sym_set_const(value, Py_True); | |
| // res = sym_new_type(ctx, &PyBool_Type); | |
| // } |
Sorry, something went wrong.
| // 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.
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); |
Sorry, something went wrong.
|
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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request. |
Sorry, something went wrong.
…e-130415.ibOV6B.rst Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
|
Thanks for the contribution. Unfortunately this has a critical flaw. |
Sorry, something went wrong.
@brandtbucher
Thanks for the help.