Skip to content

Commit

Permalink
Don't bother deduplicating (and factor out some repeated code)
Browse files Browse the repository at this point in the history
  • Loading branch information
@brandtbucher
brandtbucher committed Aug 28, 2024
1 parent 06fffd7 commit 2d4d2a8
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 233 deletions.
2 changes: 1 addition & 1 deletion Include/internal/pycore_optimizer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ PyAPI_FUNC(void) _Py_Executors_InvalidateAll(PyInterpreterState *interp, int is_

int _Py_uop_analyze_and_optimize(struct _PyInterpreterFrame *frame,
_PyUOpInstruction *trace, int trace_len, int curr_stackentries,
_PyBloomFilter *dependencies, PyObject *refs);
_PyBloomFilter *dependencies, PyObject *new_refs);

extern PyTypeObject _PyCounterExecutor_Type;
extern PyTypeObject _PyCounterOptimizer_Type;
Expand Down
87 changes: 21 additions & 66 deletions Python/optimizer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1132,47 +1132,6 @@ sanity_check(_PyExecutorObject *executor)
#undef CHECK
#endif

static PyObject *
safe_constant_key(PyObject *o)
{
PyObject *k = _PyCode_ConstantKey(o);
// A key of tuple[int, object] is used for "other" constants, which may
// include arbitrary objects. We don't want to try to hash them or check
// their equality, so just make the key a tuple[int] (their address):
if (k && PyTuple_CheckExact(k) && PyLong_CheckExact(PyTuple_GET_ITEM(k, 0))) {
Py_SETREF(k, PyTuple_Pack(1, PyTuple_GET_ITEM(k, 0)));
}
return k;
}

static bool
safe_contains(PyObject *refs, PyObject *o)
{
assert(PyList_CheckExact(refs));
for (int i = 0; i < PyList_GET_SIZE(refs); i++) {
if (Py_Is(o, PyList_GET_ITEM(refs, i))) {
return true;
}
}
return false;
}

static int
merge_const(PyObject *refs, PyObject **o_p)
{
assert(PyDict_CheckExact(refs));
assert(!_Py_IsImmortal(*o_p));
PyObject *o = *o_p;
PyObject *k = safe_constant_key(o);
if (k == NULL) {
return -1;
}
int res = PyDict_SetDefaultRef(refs, k, o, o_p);
Py_DECREF(k);
Py_DECREF(o);
return res;
}

/* Makes an executor from a buffer of uops.
* Account for the buffer having gaps and NOPs by computing a "used"
* bit vector and only copying the used uops. Here "used" means reachable
Expand Down Expand Up @@ -1287,11 +1246,13 @@ uop_optimize(
}
assert(length < UOP_MAX_TRACE_LENGTH);
OPT_STAT_INC(traces_created);
char *env_var = Py_GETENV("PYTHON_UOPS_OPTIMIZE");
// These are any references that were created during optimization, and need
// to be kept alive until we build the executor's refs tuple:
PyObject *new_refs = PyList_New(0);
if (new_refs == NULL) {
return -1;
}
char *env_var = Py_GETENV("PYTHON_UOPS_OPTIMIZE");
if (env_var == NULL || *env_var == '\0' || *env_var > '0') {
length = _Py_uop_analyze_and_optimize(frame, buffer,
length,
Expand Down Expand Up @@ -1319,45 +1280,39 @@ uop_optimize(
assert(_PyOpcode_uop_name[buffer[pc].opcode]);
assert(strncmp(_PyOpcode_uop_name[buffer[pc].opcode], _PyOpcode_uop_name[opcode], strlen(_PyOpcode_uop_name[opcode])) == 0);
}
PyObject *used_refs = PyDict_New();
if (used_refs == NULL) {
// We *might* want to de-duplicate these. In addition to making sure we do
// so in a way that preserves "equal" constants with different types (see
// _PyCode_ConstantKey), we *also* need to be careful to compare unknown
// objects by identity, since we don't want to invoke arbitary code in a
// __hash__/__eq__ implementation. It might be more trouble than it's worth:
int refs_needed = 0;
for (int i = 0; i < length; i++) {
if (buffer[i].opcode == _LOAD_CONST_INLINE) {
refs_needed++;
}
}
PyObject *refs = PyTuple_New(refs_needed);
if (refs == NULL) {
Py_DECREF(new_refs);
return -1;
}
int j = 0;
for (int i = 0; i < length; i++) {
if (buffer[i].opcode == _LOAD_CONST_INLINE) {
PyObject **o_p = (PyObject **)&buffer[i].operand;
if (!safe_contains(new_refs, *o_p)) {
continue;
}
Py_INCREF(*o_p);
int err = merge_const(used_refs, o_p);
Py_DECREF(*o_p);
if (err < 0) {
Py_DECREF(used_refs);
Py_DECREF(new_refs);
return -1;
}
PyTuple_SET_ITEM(refs, j++, Py_NewRef(buffer[i].operand));
}
}
Py_DECREF(new_refs);
Py_SETREF(used_refs, PyDict_Values(used_refs));
if (used_refs == NULL) {
return -1;
}
Py_SETREF(used_refs, PyList_AsTuple(used_refs));
if (used_refs == NULL) {
return -1;
}
assert(j == refs_needed);
OPT_HIST(effective_trace_length(buffer, length), optimized_trace_length_hist);
length = prepare_for_execution(buffer, length);
assert(length <= UOP_MAX_TRACE_LENGTH);
_PyExecutorObject *executor = make_executor_from_uops(buffer, length, &dependencies);
if (executor == NULL) {
Py_DECREF(used_refs);
Py_DECREF(refs);
return -1;
}
executor->refs = used_refs;
executor->refs = refs;
assert(length <= UOP_MAX_TRACE_LENGTH);
*exec_ptr = executor;
return 1;
Expand Down
18 changes: 14 additions & 4 deletions Python/optimizer_analysis.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,20 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer,

#define GETLOCAL(idx) ((ctx->frame->locals[idx]))

#define REPLACE_OP(INST, OP, ARG, OPERAND) \
(INST)->opcode = OP; \
(INST)->oparg = ARG; \
(INST)->operand = OPERAND;
#define REPLACE_OP(INST, OP, ARG, OPERAND) \
do { \
(INST)->opcode = (OP); \
(INST)->oparg = (ARG); \
(INST)->operand = (OPERAND); \
} while (0)

#define REPLACE_OP_WITH_LOAD_CONST(INST, CONST) \
do { \
PyObject *o = (CONST); \
int opcode = _Py_IsImmortal(o) ? _LOAD_CONST_INLINE_BORROW \
: _LOAD_CONST_INLINE; \
REPLACE_OP((INST), opcode, 0, (uintptr_t)o); \
} while (0)

/* Shortened forms for convenience, used in optimizer_bytecodes.c */
#define sym_is_not_null _Py_uop_sym_is_not_null
Expand Down
117 changes: 36 additions & 81 deletions Python/optimizer_bytecodes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,7 @@ dummy_func(void) {
op(_LOAD_FAST, (-- value)) {
value = GETLOCAL(oparg);
if (sym_is_const(value)) {
PyObject *val = sym_get_const(value);
int opcode = _Py_IsImmortal(val) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)val);
REPLACE_OP_WITH_LOAD_CONST(this_instr, sym_get_const(value));
}
}

Expand Down Expand Up @@ -199,20 +197,15 @@ dummy_func(void) {
assert(PyLong_CheckExact(sym_get_const(right)));
PyObject *temp = _PyLong_Add((PyLongObject *)sym_get_const(left),
(PyLongObject *)sym_get_const(right));
if (temp == NULL) {
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
goto error;
}
assert(this_instr[-2].opcode == _NOP);
assert(this_instr[-1].opcode == _NOP);
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
if (PyList_Append(new_refs, temp)) {
goto error;
}
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
}
else {
res = sym_new_type(ctx, &PyLong_Type);
Expand All @@ -227,20 +220,15 @@ dummy_func(void) {
assert(PyLong_CheckExact(sym_get_const(right)));
PyObject *temp = _PyLong_Subtract((PyLongObject *)sym_get_const(left),
(PyLongObject *)sym_get_const(right));
if (temp == NULL) {
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
goto error;
}
assert(this_instr[-2].opcode == _NOP);
assert(this_instr[-1].opcode == _NOP);
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
if (PyList_Append(new_refs, temp)) {
goto error;
}
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
}
else {
res = sym_new_type(ctx, &PyLong_Type);
Expand All @@ -255,20 +243,15 @@ dummy_func(void) {
assert(PyLong_CheckExact(sym_get_const(right)));
PyObject *temp = _PyLong_Multiply((PyLongObject *)sym_get_const(left),
(PyLongObject *)sym_get_const(right));
if (temp == NULL) {
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
goto error;
}
assert(this_instr[-2].opcode == _NOP);
assert(this_instr[-1].opcode == _NOP);
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
if (PyList_Append(new_refs, temp)) {
goto error;
}
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
}
else {
res = sym_new_type(ctx, &PyLong_Type);
Expand All @@ -284,20 +267,15 @@ dummy_func(void) {
PyObject *temp = PyFloat_FromDouble(
PyFloat_AS_DOUBLE(sym_get_const(left)) +
PyFloat_AS_DOUBLE(sym_get_const(right)));
if (temp == NULL) {
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
goto error;
}
assert(this_instr[-2].opcode == _NOP);
assert(this_instr[-1].opcode == _NOP);
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
if (PyList_Append(new_refs, temp)) {
goto error;
}
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
}
else {
res = sym_new_type(ctx, &PyFloat_Type);
Expand All @@ -313,20 +291,15 @@ dummy_func(void) {
PyObject *temp = PyFloat_FromDouble(
PyFloat_AS_DOUBLE(sym_get_const(left)) -
PyFloat_AS_DOUBLE(sym_get_const(right)));
if (temp == NULL) {
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
goto error;
}
assert(this_instr[-2].opcode == _NOP);
assert(this_instr[-1].opcode == _NOP);
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
if (PyList_Append(new_refs, temp)) {
goto error;
}
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
}
else {
res = sym_new_type(ctx, &PyFloat_Type);
Expand All @@ -342,20 +315,15 @@ dummy_func(void) {
PyObject *temp = PyFloat_FromDouble(
PyFloat_AS_DOUBLE(sym_get_const(left)) *
PyFloat_AS_DOUBLE(sym_get_const(right)));
if (temp == NULL) {
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
goto error;
}
assert(this_instr[-2].opcode == _NOP);
assert(this_instr[-1].opcode == _NOP);
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
if (PyList_Append(new_refs, temp)) {
goto error;
}
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
}
else {
res = sym_new_type(ctx, &PyFloat_Type);
Expand All @@ -366,20 +334,15 @@ dummy_func(void) {
if (sym_is_const(left) && sym_is_const(right) &&
sym_matches_type(left, &PyUnicode_Type) && sym_matches_type(right, &PyUnicode_Type)) {
PyObject *temp = PyUnicode_Concat(sym_get_const(left), sym_get_const(right));
if (temp == NULL) {
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
goto error;
}
assert(this_instr[-2].opcode == _NOP);
assert(this_instr[-1].opcode == _NOP);
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
REPLACE_OP(&this_instr[-1], _POP_TOP, 0, 0);
if (PyList_Append(new_refs, temp)) {
goto error;
}
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)temp);
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
REPLACE_OP(this_instr - 1, _POP_TOP, 0, 0);
REPLACE_OP_WITH_LOAD_CONST(this_instr, temp);
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
}
else {
res = sym_new_type(ctx, &PyUnicode_Type);
Expand All @@ -391,22 +354,17 @@ dummy_func(void) {
if (sym_is_const(left) && sym_is_const(right) &&
sym_matches_type(left, &PyUnicode_Type) && sym_matches_type(right, &PyUnicode_Type)) {
PyObject *temp = PyUnicode_Concat(sym_get_const(left), sym_get_const(right));
if (temp == NULL) {
if (temp == NULL || _PyList_AppendTakeRef((PyListObject *)new_refs, temp)) {
goto error;
}
assert(this_instr[-3].opcode == _NOP);
assert(this_instr[-2].opcode == _NOP);
assert(this_instr[-1].opcode == _NOP);
REPLACE_OP(&this_instr[-3], _POP_TOP, 0, 0);
REPLACE_OP(&this_instr[-2], _POP_TOP, 0, 0);
if (PyList_Append(new_refs, temp)) {
goto error;
}
int opcode = _Py_IsImmortal(temp) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(&this_instr[-1], opcode, 0, (uintptr_t)temp);
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
REPLACE_OP(this_instr - 3, _POP_TOP, 0, 0);
REPLACE_OP(this_instr - 2, _POP_TOP, 0, 0);
REPLACE_OP_WITH_LOAD_CONST(this_instr - 1, temp);
REPLACE_OP(this_instr, _STORE_FAST, this_instr->operand, 0);
res = sym_new_const(ctx, temp);
}
else {
res = sym_new_type(ctx, &PyUnicode_Type);
Expand Down Expand Up @@ -505,8 +463,7 @@ dummy_func(void) {

op(_LOAD_CONST, (-- value)) {
PyObject *val = PyTuple_GET_ITEM(co->co_consts, this_instr->oparg);
int opcode = _Py_IsImmortal(val) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)val);
REPLACE_OP_WITH_LOAD_CONST(this_instr, val);
value = sym_new_const(ctx, val);
}

Expand All @@ -530,9 +487,7 @@ dummy_func(void) {

op(_COPY, (bottom, unused[oparg-1] -- bottom, unused[oparg-1], top)) {
if (sym_is_const(bottom)) {
PyObject *value = sym_get_const(bottom);
int opcode = _Py_IsImmortal(value) ? _LOAD_CONST_INLINE_BORROW : _LOAD_CONST_INLINE;
REPLACE_OP(this_instr, opcode, 0, (uintptr_t)value);
REPLACE_OP_WITH_LOAD_CONST(this_instr, sym_get_const(bottom));
}
assert(oparg > 0);
top = bottom;
Expand Down
Loading

0 comments on commit 2d4d2a8

Please sign in to comment.