GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — GH-130415: Optimize JIT path for _TO_BOOL_INT branching by Klaus117 · Pull Request #130477 · python/cpython Conversation
|
|
All commit authors signed the Contributor License Agreement. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
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 |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| 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 rename f in this function to zero?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| # 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?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Uh oh!
There was an error while loading. Please reload this page.
| // 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); | |
| // } |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| // 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); |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
|
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 |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…e-130415.ibOV6B.rst Co-authored-by: Brandt Bucher <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks for the contribution. Unfortunately this has a critical flaw. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Closing in favor of #130477. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
Labels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
3 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.