From 76793afc630854b6e7037f41973006e76d7a6e74 Mon Sep 17 00:00:00 2001 From: "keenan.l" Date: Sat, 22 Feb 2025 14:45:57 -0800 Subject: [PATCH 1/5] optimize for int=0 branching. move to correct branch --- Lib/test/test_capi/test_opt.py | 61 ++++++++++++++++++++++++++++++++++ Python/optimizer_bytecodes.c | 17 ++++++++++ Python/optimizer_cases.c.h | 18 ++++++++++ 3 files changed, 96 insertions(+) diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 2a9b777862c84a..5ed86af3e0027b 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -1437,6 +1437,67 @@ def crash_addition(): crash_addition() + def test_narrow_type_to_constant_int_zero(self): + def f(n): + 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) + trace.append("A") + if not f: # Kept. + trace.append("B") + if not f: # Removed! + trace.append("C") + trace.append("D") + if f: # Removed! + trace.append("X") + trace.append("E") + trace.append("F") + if 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"), 1) + self.assertEqual(uops.count("_GUARD_IS_TRUE_POP"), 0) + # But all of the appends we care about are still there: + self.assertEqual(uops.count("_CALL_LIST_APPEND"), len("ABCDEFG")) + + # 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")) def global_identity(x): return x diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 41eb59c931aaa7..69ccbc0769cae8 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -407,6 +407,23 @@ dummy_func(void) { sym_set_type(value, &PyLong_Type); res = sym_new_type(ctx, &PyBool_Type); } + if (!sym_is_const(value)) { + assert(sym_matches_type(value, &PyLong_Type)); + int next_opcode = (this_instr + 1)->opcode; + assert(next_opcode == _CHECK_VALIDITY_AND_SET_IP); + next_opcode = (this_instr + 2)->opcode; + // If the next uop is a guard, we can narrow value. However, we + // *can't* narrow res, since that would cause the guard to be + // 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); + } + // else if (next_opcode == _GUARD_IS_TRUE_POP) { + // sym_set_const(value, Py_True); + // res = sym_new_type(ctx, &PyBool_Type); + // } + } } op(_TO_BOOL_LIST, (value -- res)) { diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index fd8486785ed8db..608d9a8635acaf 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -177,6 +177,24 @@ sym_set_type(value, &PyLong_Type); res = sym_new_type(ctx, &PyBool_Type); } + if (!sym_is_const(value)) { + assert(sym_matches_type(value, &PyLong_Type)); + int next_opcode = (this_instr + 1)->opcode; + assert(next_opcode == _CHECK_VALIDITY_AND_SET_IP); + next_opcode = (this_instr + 2)->opcode; + // If the next uop is a guard, we can narrow value. However, we + // *can't* narrow res, since that would cause the guard to be + // removed and the narrowed value to be invalid: + if (next_opcode == _GUARD_IS_FALSE_POP) { + stack_pointer[-1] = res; + sym_set_const(value, Py_GetConstant(Py_CONSTANT_ZERO)); + res = sym_new_type(ctx, &PyLong_Type); + } + // else if (next_opcode == _GUARD_IS_TRUE_POP) { + // sym_set_const(value, Py_True); + // res = sym_new_type(ctx, &PyBool_Type); + // } + } stack_pointer[-1] = res; break; } From c647e39f9d8ab1a7e4aab9548687ea88bf5d6ee4 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 22 Feb 2025 23:03:14 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst new file mode 100644 index 00000000000000..636b2211b724ec --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst @@ -0,0 +1 @@ +Improve JIT understanding of integers in boolean context. From a1139cfd4f6bbb26a71a357e20b1d325772dcfb9 Mon Sep 17 00:00:00 2001 From: Klaus117 Date: Sat, 22 Feb 2025 15:17:49 -0800 Subject: [PATCH 3/5] Update Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst Co-authored-by: Brandt Bucher --- .../2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst index 636b2211b724ec..e85760a2501c93 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-23-03-13.gh-issue-130415.ibOV6B.rst @@ -1 +1 @@ -Improve JIT understanding of integers in boolean context. +Improve the JIT's understanding of integers in boolean contexts. From b1702f20590cbca7482bf9db238dcaa88120e51b Mon Sep 17 00:00:00 2001 From: "keenan.l" Date: Sat, 22 Feb 2025 15:25:10 -0800 Subject: [PATCH 4/5] address code review. remove commented code. better names. --- Lib/test/test_capi/test_opt.py | 42 +++++----------------------------- Python/optimizer_bytecodes.c | 6 +---- 2 files changed, 7 insertions(+), 41 deletions(-) diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 5ed86af3e0027b..d06bf2f5112fe6 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -1441,19 +1441,19 @@ def test_narrow_type_to_constant_int_zero(self): def f(n): 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) + # zero is always (int) 0, but we can only prove that it's a integer: + zero = i - i # at this point python knows f is an int, but doesnt know that it is 0 (we know it is 0 though) trace.append("A") - if not f: # Kept. + if not zero: # Kept. trace.append("B") - if not f: # Removed! + if not zero: # Removed! trace.append("C") trace.append("D") - if f: # Removed! + if zero: # Removed! trace.append("X") trace.append("E") trace.append("F") - if f: # Removed! + if zero: # Removed! trace.append("X") trace.append("G") return trace @@ -1468,36 +1468,6 @@ def f(n): # But all of the appends we care about are still there: self.assertEqual(uops.count("_CALL_LIST_APPEND"), len("ABCDEFG")) - # 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")) def global_identity(x): return x diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 69ccbc0769cae8..ffb080f2d88535 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -417,12 +417,8 @@ dummy_func(void) { // 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); + res = sym_new_type(ctx, &PyBool_Type); } - // else if (next_opcode == _GUARD_IS_TRUE_POP) { - // sym_set_const(value, Py_True); - // res = sym_new_type(ctx, &PyBool_Type); - // } } } From debc65dc8a49909ee3c95eb19f7d47ee83ba9bca Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Sat, 22 Feb 2025 15:50:33 -0800 Subject: [PATCH 5/5] Remove redundant comment --- Lib/test/test_capi/test_opt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index d06bf2f5112fe6..44cfb8685e98a8 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -1442,7 +1442,7 @@ def f(n): trace = [] for i in range(n): # zero is always (int) 0, but we can only prove that it's a integer: - zero = i - i # at this point python knows f is an int, but doesnt know that it is 0 (we know it is 0 though) + zero = i - i trace.append("A") if not zero: # Kept. trace.append("B")