Skip to content
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

gh-125063: Emit slices as constants in the bytecode compiler #125064

Merged
merged 7 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Include/internal/pycore_magic_number.h
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ Known values:
Python 3.14a1 3605 (Move ENTER_EXECUTOR to opcode 255)
Python 3.14a1 3606 (Specialize CALL_KW)
Python 3.14a1 3607 (Add pseudo instructions JUMP_IF_TRUE/FALSE)
Python 3.14a1 3608 (Add support for slices)
Python 3.15 will start with 3650
Expand All @@ -271,7 +272,7 @@ PC/launcher.c must also be updated.
*/

#define PYC_MAGIC_NUMBER 3607
#define PYC_MAGIC_NUMBER 3608
/* This is equivalent to converting PYC_MAGIC_NUMBER to 2 bytes
(little-endian) and then appending b'\r\n'. */
#define PYC_MAGIC_NUMBER_TOKEN \
Expand Down
27 changes: 25 additions & 2 deletions Lib/test/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,14 @@ def check_op_count(func, op, expected):
actual += 1
self.assertEqual(actual, expected)

def check_consts(func, typ, expected):
slice_consts = 0
consts = func.__code__.co_consts
for instr in dis.Bytecode(func):
if instr.opname == "LOAD_CONST" and isinstance(consts[instr.oparg], typ):
slice_consts += 1
self.assertEqual(slice_consts, expected)

def load():
return x[a:b] + x [a:] + x[:b] + x[:]

Expand All @@ -1388,15 +1396,30 @@ def long_slice():
def aug():
x[a:b] += y

check_op_count(load, "BINARY_SLICE", 4)
def aug_const():
x[1:2] += y

def compound_const_slice():
x[1:2:3, 4:5:6] = y

check_op_count(load, "BINARY_SLICE", 3)
check_op_count(load, "BUILD_SLICE", 0)
check_op_count(store, "STORE_SLICE", 4)
check_consts(load, slice, 1)
check_op_count(store, "STORE_SLICE", 3)
check_op_count(store, "BUILD_SLICE", 0)
check_consts(store, slice, 1)
check_op_count(long_slice, "BUILD_SLICE", 1)
check_op_count(long_slice, "BINARY_SLICE", 0)
check_op_count(aug, "BINARY_SLICE", 1)
check_op_count(aug, "STORE_SLICE", 1)
check_op_count(aug, "BUILD_SLICE", 0)
check_op_count(aug_const, "BINARY_SLICE", 0)
check_op_count(aug_const, "STORE_SLICE", 0)
check_consts(aug_const, slice, 1)
check_op_count(compound_const_slice, "BINARY_SLICE", 0)
check_op_count(compound_const_slice, "BUILD_SLICE", 0)
check_consts(compound_const_slice, slice, 0)
check_consts(compound_const_slice, tuple, 1)

def test_compare_positions(self):
for opname_prefix, op in [
Expand Down
1 change: 1 addition & 0 deletions Objects/codeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -2336,6 +2336,7 @@ _PyCode_ConstantKey(PyObject *op)
if (op == Py_None || op == Py_Ellipsis
|| PyLong_CheckExact(op)
|| PyUnicode_CheckExact(op)
|| PySlice_Check(op)
/* code_richcompare() uses _PyCode_ConstantKey() internally */
|| PyCode_Check(op))
{
Expand Down
2 changes: 1 addition & 1 deletion Objects/sliceobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ Create a slice object. This is used for extended slicing (e.g. a[0:10:2]).");
static void
slice_dealloc(PySliceObject *r)
{
_PyObject_GC_UNTRACK(r);
PyObject_GC_UnTrack(r);
Py_DECREF(r->step);
Py_DECREF(r->start);
Py_DECREF(r->stop);
Expand Down
76 changes: 58 additions & 18 deletions Python/codegen.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ static int codegen_visit_expr(compiler *, expr_ty);
static int codegen_augassign(compiler *, stmt_ty);
static int codegen_annassign(compiler *, stmt_ty);
static int codegen_subscript(compiler *, expr_ty);
static int codegen_slice_two_parts(compiler *, expr_ty);
static int codegen_slice(compiler *, expr_ty);

static bool are_all_items_const(asdl_expr_seq *, Py_ssize_t, Py_ssize_t);
Expand Down Expand Up @@ -5005,12 +5006,8 @@ codegen_visit_expr(compiler *c, expr_ty e)
}
break;
case Slice_kind:
{
int n = codegen_slice(c, e);
RETURN_IF_ERROR(n);
ADDOP_I(c, loc, BUILD_SLICE, n);
RETURN_IF_ERROR(codegen_slice(c, e));
break;
}
case Name_kind:
return codegen_nameop(c, loc, e->v.Name.id, e->v.Name.ctx);
/* child nodes of List and Tuple will have expr_context set */
Expand All @@ -5023,9 +5020,22 @@ codegen_visit_expr(compiler *c, expr_ty e)
}

static bool
is_two_element_slice(expr_ty s)
is_constant_slice(expr_ty s)
{
return s->kind == Slice_kind &&
(s->v.Slice.lower == NULL ||
s->v.Slice.lower->kind == Constant_kind) &&
(s->v.Slice.upper == NULL ||
s->v.Slice.upper->kind == Constant_kind) &&
(s->v.Slice.step == NULL ||
s->v.Slice.step->kind == Constant_kind);
}

static bool
should_apply_two_element_slice_optimization(expr_ty s)
{
return !is_constant_slice(s) &&
s->kind == Slice_kind &&
s->v.Slice.step == NULL;
}

Expand All @@ -5046,8 +5056,8 @@ codegen_augassign(compiler *c, stmt_ty s)
break;
case Subscript_kind:
VISIT(c, expr, e->v.Subscript.value);
if (is_two_element_slice(e->v.Subscript.slice)) {
RETURN_IF_ERROR(codegen_slice(c, e->v.Subscript.slice));
if (should_apply_two_element_slice_optimization(e->v.Subscript.slice)) {
RETURN_IF_ERROR(codegen_slice_two_parts(c, e->v.Subscript.slice));
ADDOP_I(c, loc, COPY, 3);
ADDOP_I(c, loc, COPY, 3);
ADDOP_I(c, loc, COPY, 3);
Expand Down Expand Up @@ -5084,7 +5094,7 @@ codegen_augassign(compiler *c, stmt_ty s)
ADDOP_NAME(c, loc, STORE_ATTR, e->v.Attribute.attr, names);
break;
case Subscript_kind:
if (is_two_element_slice(e->v.Subscript.slice)) {
if (should_apply_two_element_slice_optimization(e->v.Subscript.slice)) {
ADDOP_I(c, loc, SWAP, 4);
ADDOP_I(c, loc, SWAP, 3);
ADDOP_I(c, loc, SWAP, 2);
Expand Down Expand Up @@ -5231,8 +5241,10 @@ codegen_subscript(compiler *c, expr_ty e)
}

VISIT(c, expr, e->v.Subscript.value);
if (is_two_element_slice(e->v.Subscript.slice) && ctx != Del) {
RETURN_IF_ERROR(codegen_slice(c, e->v.Subscript.slice));
if (should_apply_two_element_slice_optimization(e->v.Subscript.slice) &&
ctx != Del
) {
RETURN_IF_ERROR(codegen_slice_two_parts(c, e->v.Subscript.slice));
if (ctx == Load) {
ADDOP(c, loc, BINARY_SLICE);
}
Expand All @@ -5254,15 +5266,9 @@ codegen_subscript(compiler *c, expr_ty e)
return SUCCESS;
}

/* Returns the number of the values emitted,
* thus are needed to build the slice, or -1 if there is an error. */
static int
codegen_slice(compiler *c, expr_ty s)
codegen_slice_two_parts(compiler *c, expr_ty s)
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
{
int n = 2;
assert(s->kind == Slice_kind);

/* only handles the cases where BUILD_SLICE is emitted */
if (s->v.Slice.lower) {
VISIT(c, expr, s->v.Slice.lower);
}
Expand All @@ -5277,10 +5283,44 @@ codegen_slice(compiler *c, expr_ty s)
ADDOP_LOAD_CONST(c, LOC(s), Py_None);
}

return 0;
}

/* Returns the number of the values emitted as part of the slice.
* May be 0 if a constant slice was emitted.
* -1 if there is an error. */
static int
codegen_slice(compiler *c, expr_ty s)
{
int n = 2;
assert(s->kind == Slice_kind);

if (is_constant_slice(s)) {
PyObject *start = NULL;
if (s->v.Slice.lower) {
start = s->v.Slice.lower->v.Constant.value;
}
PyObject *stop = NULL;
if (s->v.Slice.upper) {
stop = s->v.Slice.upper->v.Constant.value;
}
PyObject *step = NULL;
if (s->v.Slice.step) {
step = s->v.Slice.step->v.Constant.value;
}
PyObject *slice = PySlice_New(start, stop, step);
ADDOP_LOAD_CONST_NEW(c, LOC(s), slice);
return 0;
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
}

RETURN_IF_ERROR(codegen_slice_two_parts(c, s));

if (s->v.Slice.step) {
n++;
VISIT(c, expr, s->v.Slice.step);
}

ADDOP_I(c, LOC(s), BUILD_SLICE, n);
return n;
}

Expand Down
34 changes: 34 additions & 0 deletions Python/marshal.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ module marshal
#define TYPE_UNKNOWN '?'
#define TYPE_SET '<'
#define TYPE_FROZENSET '>'
#define TYPE_SLICE ':'
#define FLAG_REF '\x80' /* with a type, add obj to index */

#define TYPE_ASCII 'a'
Expand Down Expand Up @@ -613,6 +614,13 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
w_pstring(view.buf, view.len, p);
PyBuffer_Release(&view);
}
else if (PySlice_Check(v)) {
PySliceObject *slice = (PySliceObject *)v;
W_TYPE(TYPE_SLICE, p);
w_object(slice->start, p);
w_object(slice->stop, p);
w_object(slice->step, p);
}
else {
W_TYPE(TYPE_UNKNOWN, p);
p->error = WFERR_UNMARSHALLABLE;
Expand Down Expand Up @@ -1534,6 +1542,32 @@ r_object(RFILE *p)
retval = Py_NewRef(v);
break;

case TYPE_SLICE:
{
Py_ssize_t idx = r_ref_reserve(flag, p);
PyObject *stop = NULL;
PyObject *step = NULL;
PyObject *start = r_object(p);
if (start == NULL) {
goto cleanup;
}
stop = r_object(p);
if (stop == NULL) {
goto cleanup;
}
step = r_object(p);
if (step == NULL) {
goto cleanup;
}
retval = PySlice_New(start, stop, step);
r_ref_insert(retval, idx, flag, p);
cleanup:
Py_XDECREF(start);
Py_XDECREF(stop);
Py_XDECREF(step);
break;
}

default:
/* Bogus data got written, which isn't ideal.
This will let you keep working and recover. */
Expand Down
Loading