Skip to content

gh-98831: Support conditional effects; use for LOAD_ATTR #101333

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Jan 30, 2023
Merged

gh-98831: Support conditional effects; use for LOAD_ATTR #101333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
af97a7e
Add parser support for conditional stack effects
gvanrossum Jan 25, 2023
fbcdde3
Fix a typo in test_generator.py that made it into main :-(
gvanrossum Jan 25, 2023
27246e9
Make conditional effects work, in theory
gvanrossum Jan 25, 2023
667630e
Convert LOAD_ATTR to new format
gvanrossum Jan 26, 2023
69dbc5e
Conditionalize effect_size()
gvanrossum Jan 26, 2023
6a03438
Fix two bugs in case generator
gvanrossum Jan 26, 2023
2310916
Merge branch 'fix-op-metadata' into cond-cases
gvanrossum Jan 26, 2023
a0fa513
Blackify
gvanrossum Jan 26, 2023
8aeaf01
Merge remote-tracking branch 'origin/main' into cond-cases
gvanrossum Jan 26, 2023
6240934
Use Irit's idea
gvanrossum Jan 26, 2023
d78e2a7
Cleanup
gvanrossum Jan 26, 2023
e69c1f3
Change order in LOAD_ATTR output effect
gvanrossum Jan 27, 2023
c5529c6
Fix type annotations to satisfy pyright (and mypy mostly)
gvanrossum Jan 28, 2023
37e3b40
Solve uninitalized variable warning using = NULL
gvanrossum Jan 28, 2023
81e5236
Use 'X ? 1 : 0' instead of 'X != 0'
gvanrossum Jan 28, 2023
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
38 changes: 13 additions & 25 deletions Python/bytecodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@

// Dummy variables for stack effects.
static PyObject *value, *value1, *value2, *left, *right, *res, *sum, *prod, *sub;
static PyObject *container, *start, *stop, *v, *lhs, *rhs;
static PyObject *container, *start, *stop, *v, *lhs, *rhs, *res2;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will also need static int res2_exists, and a bunch of other things, before I land this.

static PyObject *list, *tuple, *dict, *owner, *set, *str, *tup, *map, *keys;
static PyObject *exit_func, *lasti, *val, *retval, *obj, *iter;
static PyObject *aiter, *awaitable, *iterable, *w, *exc_value, *bc;
Expand Down Expand Up @@ -1438,13 +1438,11 @@ dummy_func(
PREDICT(JUMP_BACKWARD);
}

// error: LOAD_ATTR has irregular stack effect
inst(LOAD_ATTR) {
inst(LOAD_ATTR, (unused/9, owner -- res2 if (oparg & 1), res)) {
#if ENABLE_SPECIALIZATION
_PyAttrCache *cache = (_PyAttrCache *)next_instr;
if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) {
assert(cframe.use_tracing == 0);
PyObject *owner = TOP();
PyObject *name = GETITEM(names, oparg>>1);
next_instr--;
_Py_Specialize_LoadAttr(owner, next_instr, name);
Expand All @@ -1454,26 +1452,18 @@ dummy_func(
DECREMENT_ADAPTIVE_COUNTER(cache->counter);
#endif /* ENABLE_SPECIALIZATION */
PyObject *name = GETITEM(names, oparg >> 1);
PyObject *owner = TOP();
if (oparg & 1) {
/* Designed to work in tandem with CALL. */
/* Designed to work in tandem with CALL, pushes two values. */
PyObject* meth = NULL;

int meth_found = _PyObject_GetMethod(owner, name, &meth);

if (meth == NULL) {
/* Most likely attribute wasn't found. */
goto error;
}

if (meth_found) {
if (_PyObject_GetMethod(owner, name, &meth)) {
/* We can bypass temporary bound method object.
meth is unbound method and obj is self.

meth | self | arg1 | ... | argN
*/
SET_TOP(meth);
PUSH(owner); // self
assert(meth != NULL); // No errors on this branch
res2 = meth;
res = owner; // Transfer ownership
}
else {
/* meth is not an unbound method (but a regular attr, or
Expand All @@ -1483,20 +1473,18 @@ dummy_func(

NULL | meth | arg1 | ... | argN
*/
SET_TOP(NULL);
Py_DECREF(owner);
PUSH(meth);
ERROR_IF(meth == NULL, error);
res2 = NULL;
res = meth;
}
}
else {
PyObject *res = PyObject_GetAttr(owner, name);
if (res == NULL) {
goto error;
}
/* Classic, pushes one value. */
res = PyObject_GetAttr(owner, name);
Py_DECREF(owner);
SET_TOP(res);
ERROR_IF(res == NULL, error);
}
JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR);
}

// error: LOAD_ATTR has irregular stack effect
Expand Down
40 changes: 18 additions & 22 deletions Python/generated_cases.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions Python/opcode_metadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ _PyOpcode_num_popped(int opcode, int oparg) {
case MAP_ADD:
return 2;
case LOAD_ATTR:
return -1;
return 1;
case LOAD_ATTR_INSTANCE_VALUE:
return -1;
case LOAD_ATTR_MODULE:
Expand Down Expand Up @@ -531,7 +531,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) {
case MAP_ADD:
return 0;
case LOAD_ATTR:
return -1;
return ((oparg & 1) ? 1 : 0) + 1;
case LOAD_ATTR_INSTANCE_VALUE:
return -1;
case LOAD_ATTR_MODULE:
Expand Down Expand Up @@ -694,7 +694,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) {
}
#endif
enum Direction { DIR_NONE, DIR_READ, DIR_WRITE };
enum InstructionFormat { INSTR_FMT_IB, INSTR_FMT_IBC, INSTR_FMT_IBC0, INSTR_FMT_IBC000, INSTR_FMT_IBIB, INSTR_FMT_IX, INSTR_FMT_IXC, INSTR_FMT_IXC000 };
enum InstructionFormat { INSTR_FMT_IB, INSTR_FMT_IBC, INSTR_FMT_IBC0, INSTR_FMT_IBC000, INSTR_FMT_IBC00000000, INSTR_FMT_IBIB, INSTR_FMT_IX, INSTR_FMT_IXC, INSTR_FMT_IXC000 };
struct opcode_metadata {
enum Direction dir_op1;
enum Direction dir_op2;
Expand Down Expand Up @@ -791,7 +791,7 @@ struct opcode_metadata {
[DICT_UPDATE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
[DICT_MERGE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
[MAP_ADD] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
[LOAD_ATTR] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
[LOAD_ATTR] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBC00000000 },
[LOAD_ATTR_INSTANCE_VALUE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
[LOAD_ATTR_MODULE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
[LOAD_ATTR_WITH_HINT] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
Expand Down
Loading