Skip to content

Eliminate constant inputs and output constants for simple operations #115506

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

Open
1 of 2 tasks
Tracked by #115419
Fidget-Spinner opened this issue Feb 15, 2024 · 4 comments
Open
1 of 2 tasks
Tracked by #115419
Labels
type-feature A feature request or enhancement

Comments

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Feb 15, 2024

Linked PRs

@gvanrossum
Copy link
Member

A little more detail would be helpful. Even just an example.

@Fidget-Spinner
Copy link
Member Author

Fidget-Spinner commented Feb 20, 2024

Here's the design:

At every point where we propagate constants, we also try to eliminate the last n stack input entries.
E.g. for _BINARY_OP_ADD_INT, we eliminate left and right. To create something robust, we shall autogenerate the net stack effects of all opcodes. The algorithm is as follows:

int zap_all_constants() {
    // zap_all_constants
    // We probably don't want to store non-immortal constants, as they complicate reference counting.
    if (!_PyObject_IsImmortal(const_val) {
        return 0;
    }
    stack_effect = -stack_inputs(opcode);
    _PyUopInstruction *start = curr_instr;
    while (op_is_pure(start) && stack_effect > 0) {
        stack_effect += net_stack_effect(start);
        start--;
    }
    if (stack_effect != 0) {
        return 0;
    }
    for (; start < curr_instr; start++) {
        REPLACE_OP(start, _NOP, 0, 0);
    }
    return 1;
}

// Do this outside, because some ops have more than one stack output (e.g. a PUSH_NULL):
if (zap_all_constants(...)) {
    REPLACE_OP(curr_instr, _LOAD_CONST_INLINE_BORROW, 0, const_val);
}

This also means we will never overshoot the buffer, because anything that requires the old _SHRINK_STACK won't be const propagated.

@Fidget-Spinner Fidget-Spinner self-assigned this Feb 20, 2024
@markshannon
Copy link
Member

What would the code in the redundancy elimination pass for _BINARY_OP_ADD_INT actually look like?

@markshannon
Copy link
Member

Can you make seperate issues for BINARY_OP, which is quite complex, and TO_BOOL, which is very simple in comparison?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants