New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move jumps optimization from the peepholer to the compiler #76658
Comments
The proposed patch moves jumps optimization from the peepholer to the compiler. The optimization is performed for lists of instructions before generating concrete bytecode and is not restricted by the size of bytecode instructions. It allows to optimize more long chains of jumps. This is a step to getting rid of the peepholer. |
There is no goal to get rid of the peepholer optimizer. Please don't invent this as a requirement. If some optimization are more easily performed upstream, then go ahead and move them upstream. The goal is to have a net reduction in complexity, not just the elimination of peephole.c just because you decided it should disappear. |
Actually moving any of remaining optimizations (including two optimizations that are moved by this issue and two other that are left) slightly increases the complexity. But instead the optimizations become more general. At the end, after moving all optimizations, we could drop the auxiliary code in the peepholer (building the table of basic blocks, removing NOPs, fixing up jump targets) and the net complexity will be reduced. This can also reduce the time and memory consumption of compilation. Thus getting rid of the peepholer optimizer working with the bytecode is my goal. But some peephole optimizations will work with the intermediate fixed-size representations used by the compiler, before generating the concrete bytecode. |
Then perhaps the goal should be to make the peephole operate on that intermediate representation? I think it's valuable to have a separate C module for optimizations, instead of cramming them in compiler.c. |
This would require major rewriting. PR 5077 adds just two functions in compile.c. But they use 4 structures defined locally in this file. If move these two functions in a separate file we will need to move these 4 structures and related definitions to the common header file. And maybe it will be in vain if once all these optimizations will be moved to other parts of the compiler. Some of jumps optimization already are performed on instructions emitting stage. Dead code elimination perhaps could be performed at the stage of assembling the bytecode from basic blocks. Constant tuples folding could be implemented by at least three ways besides the current implementation, I'm testing what of them is the simplest. |
Well, on the other hand, the change you're proposing is hardly necessary, unless it actually demonstrates a noticeable improvement on some workload. So perhaps we should leave the peepholer alone until someone has a better idea. |
Simplified PR 5077. It uses now NEXT_BLOCK() to guarantee that a new block is always used after jumps. NEXT_BLOCK() was defined, but not used before. bpo-35193 and bpo-9566 demonstrate that the code of peephole.c is complex and error-prone. Moving it to the upper level makes it more error-proof and general. |
After analyzing the difference between bytecodes generated by the current peepholer and the new optimizer I have found that he peepholer do not optimizes jumps in case of some multiline expressions. For example: [x 1 0 BUILD_LIST 0 2 6 STORE_FAST 1 (x) 1 12 LOAD_FAST 1 (x) if x:
if (y and
z):
foo()
else:
bar() 1 0 LOAD_NAME 0 (x) 2 4 LOAD_NAME 1 (y) 3 8 LOAD_NAME 2 (z) 2 10 POP_JUMP_IF_FALSE 18 4 12 LOAD_NAME 3 (foo) 6 >> 20 LOAD_NAME 4 (bar) It preserves jumps to jump instructions. I do not know the cause. All works with single-line expressions. The new optimizer does not have this bug. |
Opened bpo-37213 for the regression in the peepholer. |
PR 14116 is based on the part of PR 5077. It serves three functions:
|
This has rendered obsolete by removal of the peephole optimizer in https://bugs.python.org/issue41323 |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: