| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
| _REPLACE_WITH_TRUE + | ||
| POP_TOP; | ||
|
|
||
| tier2 op(_TO_BOOL_DICT, (value -- res)) { |
There was a problem hiding this comment.
You can merge this with _TO_BOOL_SIZED by using the fact that both do a fixed offset lookup.
In tier2 optimizer you can set the offset for where the size is stored and do size = (Py_ssize_t)((char *)obj + offset) and check that directly.
Sorry, something went wrong.
There was a problem hiding this comment.
I see what you mean. It goes into the internals of PyDict (e.g. not using PyDict_GET_SIZE, but doing manual offset calculations) and we also need to store the offset somewhere. So I think this is too much of a complication to get rid of a tier2 opcode.
Sorry, something went wrong.
There was a problem hiding this comment.
we also need to store the offset somewhere.
You can store it in the instruction operand0.
Sorry, something went wrong.
| REPLACE_OP(this_instr, _TO_BOOL_DICT, 0, 0); | ||
| } | ||
| else if (tp == &PyTuple_Type || | ||
| tp == &PySet_Type || |
There was a problem hiding this comment.
This is incorrect for set as it does not uses PyObject_VAR_HEAD, this works by accident because it has fill at that offset which is incorrect if set has dummy entries.
Sorry, something went wrong.
There was a problem hiding this comment.
Good catch! I updated the PR to handle the set/frozenset separately.
We can also use your suggestion to fold everything into the _TO_BOOL_SIZED. That means we have to load the offset at runtime (minor cost), but it does keep the number of ops lower. I implemented this in main...eendebakpt:to_bool_specialization_v2.
Sorry, something went wrong.
There was a problem hiding this comment.
That means we have to load the offset at runtime (minor cost), but it does keep the number of ops lower.
I don't think so, in the JIT the offset would be burned into the machine code itself so the offset is fixed and not looked up at runtime.
Sorry, something went wrong.
There was a problem hiding this comment.
The problem with recording uops not being allowed after specializing uops has been fixed, so you can add a recording uop to _TO_BOOL and use the recorded information for better specialization.
#148285
Sorry, something went wrong.
@markshannon Adding the recording uop will help to specialize for several cases. But it will slow down (a tiny bit) the unspecialized cases. Should we make the recording uop change in a followup PR? And do you have an opinion on whether to use the _TO_BOOL_DICT in this PR, or move everything to _TO_BOOL_SIZED as done in main...eendebakpt:to_bool_specialization_v2? |
Sorry, something went wrong.
Take v2's parameterized _TO_BOOL_SIZED design (with size_offset operand) which subsumes _TO_BOOL_DICT and _TO_BOOL_ANY_SET. Sets/frozensets use PySetObject.used (not Py_SIZE, which aliases the dummy-counting fill slot). Regenerated all generated files from the merged source.
# Conflicts: # Include/internal/pycore_uop_ids.h # Lib/test/test_capi/test_opt.py # Python/optimizer_bytecodes.c # Python/optimizer_cases.c.h # Python/record_functions.c.h
Earlier merge resolutions silently dropped a sizable set of tests that exist on origin/main (test_get_iter_list, test_get_iter_gen, test_get_iter_trad, test_cached_load_special, all test_binary_op_extend_*, test_contains_op_frozendict_const_fold, test_not_contains_op_frozendict_const_fold, test_load_attr_getattribute_frame, etc.). Restore the file from origin/main and re-insert the eight TO_BOOL tests that are unique to this branch (test_to_bool_kwargs_dict, _empty_dict, _varargs_tuple, _empty_tuple, _args_and_kwargs, _args_kwargs_with_regular_params, _kwargs_only_no_varargs, _set_with_dummy_entries) right after test_to_bool_always_true. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
I decided to go with the _TO_BOOL_SIZED variation. It is only a single opcode covering many types. The PR now does 3 things: add new opcodes, add a recording uop, add type information for *args/**kwargs. If needed we can split it into multiple PRs. |
Sorry, something went wrong.
# Conflicts: # Include/internal/pycore_uop_ids.h
| uops = get_opnames(ex) | ||
| self.assertNotIn("_REPLACE_WITH_TRUE", uops) | ||
|
|
||
| def test_to_bool_kwargs_dict(self): |
There was a problem hiding this comment.
These tests are too verbose, I would prefer if you combine several of them into one, not each case needs a different test.
Sorry, something went wrong.
There was a problem hiding this comment.
Done!
I also looked into refactoring the optimizer code to use a table based approach. The result is
If you want me to pull it into the branch give me a thumbs up.
Sorry, something went wrong.
|
This PR is stale because it has been open for 90 days with no activity. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
See discussion at #148113.
(description updated after refactoring)
This PR adds two tier2 opcodes for specialization of TO_BOOL. The *args and **kwargs arguments are marked in tier2 as tuple and dict, respectively. A recording uop is added to TO_BOOL.
"""TO_BOOL specialization benchmark. Covers both the static (sym_get_type) and recorded (sym_get_probable_type) paths through the JIT optimizer's _TO_BOOL rule. Static path: - kwargs dict (via **kwargs slot priming) - varargs tuple (via *args slot priming) - set literal (via _BUILD_SET) Recorded path (only reached via _RECORD_TOS_TYPE; bytes/bytearray/frozenset have no static-type construction at all): - dict / tuple / bytes / bytearray / set / frozenset passed as parameter """ import pyperf INNER_ITERS = 200 # ---- recorded-type path: value comes in as a parameter, no static type ---- def _recorded_inner(v): cnt = 0 for _ in range(INNER_ITERS): if v: cnt += 1 return cnt def to_bool_dict_recorded(n): d = {"a": 1} for _ in range(n): _recorded_inner(d) def to_bool_tuple_recorded(n): t = (1, 2, 3) for _ in range(n): _recorded_inner(t) def to_bool_bytes_true(n): b = b"hello" for _ in range(n): _recorded_inner(b) def to_bool_bytes_false(n): b = b"" for _ in range(n): _recorded_inner(b) def to_bool_bytearray(n): b = bytearray(b"hello") for _ in range(n): _recorded_inner(b) def to_bool_set_recorded(n): s = {1, 2, 3} for _ in range(n): _recorded_inner(s) def to_bool_frozenset(n): s = frozenset({1, 2, 3}) for _ in range(n): _recorded_inner(s) # ---- static-type path: kwargs / varargs slot priming ---- def _kwargs_inner(**kwargs): cnt = 0 for _ in range(INNER_ITERS): if kwargs: cnt += 1 return cnt def _varargs_inner(*args): cnt = 0 for _ in range(INNER_ITERS): if args: cnt += 1 return cnt def to_bool_kwargs_nonempty(n): for _ in range(n): _kwargs_inner(x=1, y=2) def to_bool_kwargs_empty(n): for _ in range(n): _kwargs_inner() def to_bool_varargs_nonempty(n): for _ in range(n): _varargs_inner(1, 2, 3) def to_bool_varargs_empty(n): for _ in range(n): _varargs_inner() # ---- static-type path: set literal via _BUILD_SET ---- def to_bool_set_literal(n): cnt = 0 for _ in range(n): s = {1} if s: cnt += 1 return cnt N = 500_000 runner = pyperf.Runner() runner.bench_func("to_bool_dict_recorded", to_bool_dict_recorded, N) runner.bench_func("to_bool_tuple_recorded", to_bool_tuple_recorded, N) runner.bench_func("to_bool_bytes_true", to_bool_bytes_true, N) runner.bench_func("to_bool_bytes_false", to_bool_bytes_false, N) runner.bench_func("to_bool_bytearray", to_bool_bytearray, N) runner.bench_func("to_bool_set_recorded", to_bool_set_recorded, N) runner.bench_func("to_bool_frozenset", to_bool_frozenset, N) runner.bench_func("to_bool_kwargs_nonempty", to_bool_kwargs_nonempty, N) runner.bench_func("to_bool_kwargs_empty", to_bool_kwargs_empty, N) runner.bench_func("to_bool_varargs_nonempty", to_bool_varargs_nonempty, N) runner.bench_func("to_bool_varargs_empty", to_bool_varargs_empty, N) runner.bench_func("to_bool_set_literal", to_bool_set_literal, N)