gh-114058: The Tier2 Optimizer #114059
Closed
gh-114058: The Tier2 Optimizer #114059
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fidget-Spinner
requested review from
gvanrossum and
markshannon
as code owners
Fidget-Spinner
changed the title
|
Please give me some time to write out the proper docs explaining the abstract IR this uses. |
Fidget-Spinner
changed the title
|
All tests run with uops on now passes except for the following two:
|
Maybe we should just fix the test? This sort of seems to be kicking the can down the road, since eventually tier 2 will be on by default. There's a |
|
1% slower on macOS (other platforms aren't building right now). 8% reduction in traces executed, but 3% increase in uops executed. PGO failure on Windows: PGO failure on Linux: Both look like recursion-related issues. The Windows one may be fixed on |
|
Thanks Brandt. Seems like the slowdown is due to Llike Mark said though, benchmark results aren't too important at the moment. Function inlining would be the most important optimization and that's missing from this PR, to be added in a future PR. |
|
Nevertheless it would be wise to dig deeper into what goes on with bm_nbody. It may be an important canary. :-) |
This reverts commit 2096a8a.
gvanrossum
reviewed
Feb 5, 2024
Comment on lines
+131
to
+135
| if var.name != MANGLED_NULL: | ||
| out.emit(f"{var.name} = sym_init_unknown(ctx);\n") | ||
| out.emit(f"if({var.name} == NULL) goto error;\n") | ||
| if var.type_prop: | ||
| out.emit(f"sym_set_type({var.name}, {var.type_prop[0]}, 0);\n") |
There was a problem hiding this comment.
This is an example of something that I think can be streamlined further. Instead of calling sym_init_unknown(ctx) followed by sym_set_type(...) there ought to be a single function you can call to create a symbol with a given type. Basically a bunch of convenience constructors.
Comment on lines
+15
to
+16
| // This the above + additional working space we need. | ||
| #define UOP_MAX_TRACE_WORKING_LENGTH (UOP_MAX_TRACE_LENGTH * 2) |
There was a problem hiding this comment.
I don't see this used in the code any more.
Comment on lines
+381
to
+389
| if (tp->tp_version_tag != 0) { | ||
| sym_set_type(sym, GUARD_TYPE_VERSION_TYPE, tp->tp_version_tag); | ||
| } | ||
| if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) { | ||
| PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(obj); | ||
| if(_PyDictOrValues_IsValues(dorv)) { | ||
| sym_set_type(sym, GUARD_DORV_VALUES_TYPE, 0); | ||
| } | ||
| } |
There was a problem hiding this comment.
This could also use a longer explanation. What special cases are these two if blocks covering?
Makes sense. I will close this PR and create a new one with the Mark's new DSL changes. Thanks for all your reviews! |
|
The successor PR with cleaned up history is at #115085. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #114058
This PR turns on the optimizer for all uops. The tier 2 uops optimizer contains a few parts: the abstract interpreter, the IR, and the codegen.
The abstract interpreter does the following:
Function inlining is left out for a future PR, as it's the most complex.
After analysis of the bytecode and doing all of the above, it emits optimized uops, and passes that to the executor.
When uops is enabled, **this passes the entire CPython test suite **. The significant milestone is that this is able to analyse and abstract interpret all CPython uops that we currently support. The other significant milestone is that this generates code that passes CPython's test suite.
Refleak tests will fail as well, as they need a design overhaul.
The design of this PR is here https://github.com/Fidget-Spinner/cpython_optimization_notes/blob/main/3.13/uops_optimizer.md
High level discussion here faster-cpython/ideas#648.
0-2% faster on Linux, 3% faster on macOS ARM64