GH-118095: Add dynamic exit support and FOR_ITER_GEN support to tier 2 #118279
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
Member
markshannon
commented
Apr 25, 2024
•
markshannon
commented
- Issue: Increase the number of micro-ops that we can handle in tier 2 #118095
gvanrossum
reviewed
Apr 25, 2024
Python/bytecodes.c
Outdated
| assert(next_instr - this_instr + oparg <= UINT16_MAX); | ||
| frame->return_offset = (uint16_t)(next_instr - this_instr + oparg); | ||
| DISPATCH_INLINED(gen_frame); | ||
| frame->return_offset = (uint16_t)(2 + oparg); |
There was a problem hiding this comment.
What does 2 stand for? Can you add a comment?
Python/bytecodes.c
Outdated
Comment on lines
3174
to
3176
| // The 'unused' output effect represents the return value | ||
| // (which will be pushed when the frame returns). | ||
| // It is needed so CALL_PY_EXACT_ARGS matches its family. |
There was a problem hiding this comment.
This comment is outdated -- I suggest just deleting it.
| @@ -87,6 +87,7 @@ _JIT_ENTRY(_PyInterpreterFrame *frame, PyObject **stack_pointer, PyThreadState * | |||
| PATCH_VALUE(_PyExecutorObject *, current_executor, _JIT_EXECUTOR) | |||
| int oparg; | |||
| int uopcode = _JIT_OPCODE; | |||
| _Py_CODEUNIT *next_instr; | |||
There was a problem hiding this comment.
I'd like to know what @brandtbucher thinks of this.
Python/bytecodes.c
Outdated
Comment on lines
4208
to
4209
| if (!backoff_counter_triggers(exit->temperature)) { | ||
| GOTO_TIER_ONE(target); |
There was a problem hiding this comment.
This doesn't advance the temperature. From _SIDE_EXIT, it looks like the correct idiom is
_Py_BackoffCounter temperature = exit->temperature;
if (!backoff_counter_triggers(temperature)) {
exit->temperature = advance_backoff_counter(temperature);
GOTO_TIER_ONE(target);
}
| @@ -4222,6 +4258,7 @@ dummy_func( | |||
| GOTO_UNWIND(); | |||
| } | |||
|
|
|||
|
|
|||
gvanrossum
approved these changes
Apr 26, 2024
|
The usual four failures. |
|
|
@markshannon I now get this warning: That's in here: case _FOR_ITER_GEN_FRAME: {
_PyInterpreterFrame *gen_frame;
gen_frame = sym_new_not_null(ctx);
if (gen_frame == NULL) goto out_of_space;
stack_pointer[0] = (_Py_UopsSymbol *)gen_frame;
stack_pointer += 1;
break;
}Apparently generated by the generator using the default code generation strategy. We may need to add a dummy case for (@Fidget-Spinner Or is there another way to fix this?) |
|
The optimizer bytecodes shouldnt even use the cast. I should fix that because ifs interfering with tagged ptrs too |
|
For now, we can just override it I think as a hotfix. |
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.