gh-115480: Type and constant propagation for int BINARY_OPs #115478
Conversation
Fidget-Spinner
requested review from
markshannon and
gvanrossum
as code owners
Fidget-Spinner
changed the title
There was a problem hiding this comment.
So IIUC, the only effect so far is that some guards may be eliminated, right?
I have a feeling that collapsing LOAD a; LOAD b; ADD into LOAD_CONST (a+b) should never overshoot the output buffer, but I know there are some gnarly corner cases, so fine to put that into a new PR.
Yes. |
| goto error; | ||
| } | ||
| res = sym_new_const(ctx, temp); | ||
| // TODO replace opcode with constant propagated one and add tests! |
There was a problem hiding this comment.
In future, please make a new issue for this sort of TODOs.
| if (temp == NULL) { | ||
| goto error; | ||
| } | ||
| res = sym_new_const(ctx, temp); |
| if (temp == NULL) { | ||
| goto error; | ||
| } | ||
| res = sym_new_const(ctx, temp); |
| if (temp == NULL) { | ||
| goto error; | ||
| } | ||
| res = sym_new_const(ctx, temp); |
|
I think we should add a res = sym_new_const(ctx, temp);
if (res == NULL) {
goto out_of_space;
}becomes FAIL_IF_NULL(res = sym_new_const(ctx, temp)); |
| if (is_const(left) && is_const(right)) { | ||
| assert(PyLong_CheckExact(get_const(left))); | ||
| assert(PyLong_CheckExact(get_const(right))); | ||
| PyObject *temp = _PyLong_Add((PyLongObject *)get_const(left), |
There was a problem hiding this comment.
This may leak, if temp is mortal.
Likewise for the other _BINARY_OP_... below.
Symbols hold a strong reference to constants, so there is no leak.
Small PR to add type/constant propagation for
_BINARY_OP_ADD/SUBTRACT/MULTIPLY_INT.BINARY_OP#115480