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.

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

Jump to bottom

remove NEXT_BLOCK() from compile.c #90964

Closed
iritkatriel opened this issue Feb 20, 2022 · 3 comments
Closed

remove NEXT_BLOCK() from compile.c #90964

iritkatriel opened this issue Feb 20, 2022 · 3 comments
Assignees
@iritkatriel
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@iritkatriel
Copy link
Member

BPO 46808
Nosy @markshannon, @brandtbucher, @iritkatriel
PRs
  • bpo-46808: remove NEXT_BLOCK() from compile.c #31448
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/iritkatriel'
closed_at = <Date 2022-02-28.19:44:10.449>
created_at = <Date 2022-02-20.14:48:48.148>
labels = ['interpreter-core', 'type-feature', '3.11']
title = 'remove NEXT_BLOCK() from compile.c'
updated_at = <Date 2022-02-28.19:44:10.449>
user = 'https://github.com/iritkatriel'

    bugs.python.org fields:

    activity = <Date 2022-02-28.19:44:10.449>
actor = 'iritkatriel'
assignee = 'iritkatriel'
closed = True
closed_date = <Date 2022-02-28.19:44:10.449>
closer = 'iritkatriel'
components = ['Interpreter Core']
creation = <Date 2022-02-20.14:48:48.148>
creator = 'iritkatriel'
dependencies = []
files = []
hgrepos = []
issue_num = 46808
keywords = ['patch']
message_count = 3.0
messages = ['413589', '413615', '413989']
nosy_count = 3.0
nosy_names = ['Mark.Shannon', 'brandtbucher', 'iritkatriel']
pr_nums = ['31448']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue46808'
versions = ['Python 3.11']
    @iritkatriel
    Copy link
    Member Author

    The compiler currently requires the code-generation functions to explicitly specify where basic blocks end, with a NEXT_BLOCK().

    If you get that wrong, you get an exception about "malformed control flow graph" later, in the cfg analysis stage. It is not obvious then where the error is, and it makes it difficult to make changes in the compiler.

    We can instead make the compiler implicitly create a new block when this is needed (which is after specific opcodes).

    @iritkatriel iritkatriel added the 3.11 only security fixes label Feb 20, 2022
    @iritkatriel iritkatriel self-assigned this Feb 20, 2022
    @iritkatriel iritkatriel added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement 3.11 only security fixes labels Feb 20, 2022
    @iritkatriel iritkatriel self-assigned this Feb 20, 2022
    @iritkatriel iritkatriel added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Feb 20, 2022
    @iritkatriel
    Copy link
    Member Author

    The patch in PR31448 delays the creation of an implicit block to when it is needed (do if we begin using another block before a new instruction is added, the implicit block is never created).

    I counted how many empty blocks are being detected by the optimizer during "python -m test" and this number went down from 41798 (for main) to 35405 (for my PR), which shows that it is not only a theoretical possibility but actually happens. It would be things like:

    NEXT_BLOCK()
VISIT_SEQ(c, stmt, s->v.Try.orelse);
compiler_use_next_block(c, end);

    where the orelse block is empty.

    In summary, the patch simplifies the code as well as making it more efficient.

    @markshannon
    Copy link
    Member

    New changeset c579243 by Irit Katriel in branch 'main':
    bpo-46808: remove NEXT_BLOCK() from compile.c (GH-31448)
    c579243

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @iritkatriel iritkatriel

    Labels
    3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @iritkatriel @markshannon