-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
GH-118095: Unify the behavior of tier 2 FOR_ITER
branch micro-ops
#118420
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
Changes from 6 commits
f8bd566
3bac858
b83d053
bb7efd4
983b7c8
df2792f
d2af12f
98b517d
aa35032
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,18 @@ | |
|
||
#define MAX_EXECUTORS_SIZE 256 | ||
|
||
#ifdef Py_DEBUG | ||
static int base_opcode(PyCodeObject *code, int offset) | ||
{ | ||
int opcode = _Py_GetBaseOpcode(code, offset); | ||
if (opcode == ENTER_EXECUTOR) { | ||
int oparg = _PyCode_CODE(code)[offset].op.arg; | ||
_PyExecutorObject *ex = code->co_executors->executors[oparg]; | ||
return ex->vm_data.opcode; | ||
} | ||
return opcode; | ||
} | ||
#endif | ||
|
||
static bool | ||
has_space_for_executor(PyCodeObject *code, _Py_CODEUNIT *instr) | ||
|
@@ -422,6 +434,14 @@ _PyUOp_Replacements[MAX_UOP_ID + 1] = { | |
[_FOR_ITER] = _FOR_ITER_TIER_TWO, | ||
}; | ||
|
||
static const uint8_t | ||
is_for_iter_test[MAX_UOP_ID + 1] = { | ||
[_GUARD_NOT_EXHAUSTED_RANGE] = 1, | ||
[_GUARD_NOT_EXHAUSTED_LIST] = 1, | ||
[_GUARD_NOT_EXHAUSTED_TUPLE] = 1, | ||
[_FOR_ITER_TIER_TWO] = 1, | ||
}; | ||
|
||
static const uint16_t | ||
BRANCH_TO_GUARD[4][2] = { | ||
[POP_JUMP_IF_FALSE - POP_JUMP_IF_FALSE][0] = _GUARD_IS_TRUE_POP, | ||
|
@@ -571,7 +591,6 @@ translate_bytecode_to_trace( | |
|
||
uint32_t opcode = instr->op.code; | ||
uint32_t oparg = instr->op.arg; | ||
uint32_t extended = 0; | ||
|
||
DPRINTF(2, "%d: %s(%d)\n", target, _PyOpcode_OpName[opcode], oparg); | ||
|
||
|
@@ -585,7 +604,6 @@ translate_bytecode_to_trace( | |
|
||
if (opcode == EXTENDED_ARG) { | ||
instr++; | ||
extended = 1; | ||
opcode = instr->op.code; | ||
oparg = (oparg << 8) | instr->op.arg; | ||
if (opcode == EXTENDED_ARG) { | ||
|
@@ -746,12 +764,13 @@ translate_bytecode_to_trace( | |
case OPARG_REPLACED: | ||
uop = _PyUOp_Replacements[uop]; | ||
assert(uop != 0); | ||
if (uop == _FOR_ITER_TIER_TWO) { | ||
target += 1 + INLINE_CACHE_ENTRIES_FOR_ITER + oparg + 2 + extended; | ||
assert(_PyCode_CODE(code)[target-2].op.code == END_FOR || | ||
_PyCode_CODE(code)[target-2].op.code == INSTRUMENTED_END_FOR); | ||
assert(_PyCode_CODE(code)[target-1].op.code == POP_TOP); | ||
} | ||
#ifdef Py_DEBUG | ||
uint32_t next_inst = target + 1 + INLINE_CACHE_ENTRIES_FOR_ITER + (oparg > 255); | ||
uint32_t jump_target = next_inst + oparg; | ||
assert(base_opcode(code, jump_target) == END_FOR || | ||
base_opcode(code, jump_target) == INSTRUMENTED_END_FOR); | ||
assert(base_opcode(code, jump_target+1) == POP_TOP); | ||
#endif | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably best to include the case also in |
||
break; | ||
default: | ||
fprintf(stderr, | ||
|
@@ -971,7 +990,15 @@ prepare_for_execution(_PyUOpInstruction *buffer, int length) | |
int opcode = inst->opcode; | ||
int32_t target = (int32_t)uop_get_target(inst); | ||
if (_PyUop_Flags[opcode] & (HAS_EXIT_FLAG | HAS_DEOPT_FLAG)) { | ||
if (target != current_jump_target) { | ||
int32_t jump_target = target; | ||
if (is_for_iter_test[opcode]) { | ||
/* Target the POP_TOP immediately after the END_FOR, | ||
* leaving only the iterator on the stack. */ | ||
int extended_arg = inst->oparg > 255; | ||
int32_t next_inst = target + 1 + INLINE_CACHE_ENTRIES_FOR_ITER + extended_arg; | ||
jump_target = next_inst + inst->oparg + 1; | ||
} | ||
if (jump_target != current_jump_target) { | ||
uint16_t exit_op; | ||
if (_PyUop_Flags[opcode] & HAS_EXIT_FLAG) { | ||
if (opcode == _TIER2_RESUME_CHECK) { | ||
|
@@ -984,8 +1011,8 @@ prepare_for_execution(_PyUOpInstruction *buffer, int length) | |
else { | ||
exit_op = _DEOPT; | ||
} | ||
make_exit(&buffer[next_spare], exit_op, target); | ||
current_jump_target = target; | ||
make_exit(&buffer[next_spare], exit_op, jump_target); | ||
current_jump_target = jump_target; | ||
current_jump = next_spare; | ||
next_spare++; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -164,12 +164,15 @@ _Py_uop_sym_set_const(_Py_UopsSymbol *sym, PyObject *const_val) | |
return true; | ||
} | ||
|
||
|
||
bool | ||
_Py_uop_sym_set_null(_Py_UopsSymbol *sym) | ||
{ | ||
if (_Py_uop_sym_is_not_null(sym)) { | ||
sym_set_bottom(sym); | ||
return false; | ||
} | ||
sym_set_flag(sym, IS_NULL); | ||
return !_Py_uop_sym_is_bottom(sym); | ||
return true; | ||
Comment on lines
+170
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this refactoring matter? If so, why not do the same for set_non_null below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not a refactoring. And yes, it should be applied to |
||
} | ||
|
||
bool | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.