Skip to content

bpo-45773: Stop "optimizing" certain jump patterns#29505

Merged
brandtbucher merged 6 commits into
python:mainfrom
brandtbucher:peephole-hang
Nov 11, 2021
Merged

bpo-45773: Stop "optimizing" certain jump patterns#29505
brandtbucher merged 6 commits into
python:mainfrom
brandtbucher:peephole-hang

Conversation

@brandtbucher

@brandtbucher brandtbucher commented Nov 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

@brandtbucher brandtbucher added type-bug An unexpected behavior, bug, or error needs backport to 3.10 only security fixes labels Nov 9, 2021
@brandtbucher brandtbucher changed the title bpo-43773: Stop "optimizing" certain jump patterns bpo-45773: Stop "optimizing" certain jump patterns Nov 9, 2021
@markshannon markshannon self-assigned this Nov 10, 2021
markshannon
markshannon requested changes Nov 10, 2021
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure about the comments // It's okay if inst->i_target == target->i_target., or the asserts assert(inst->i_target != target->i_target);

I'm not saying that they are incorrect, but it is not obvious why they are true.
Could you elaborate?

Something else to consider is to create a jump_thread function which gets all the checks if one place (different targets, compatible line numbers). It should return 1 if successful.

We could then replace code from line 8253 to 8256 with i -= jump_thread(inst, target);

@bedevere-bot

bedevere-bot commented Nov 10, 2021

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be poked with soft cushions!

@brandtbucher

brandtbucher commented Nov 11, 2021

Copy link
Copy Markdown
Member Author

I figured I'd give your jump_thread suggestion a stab, and that sort of led to a refactoring of all jump elimination. I do really like the way the logic here cleans up as a result.

If you think this is outside the scope of this PR, though, I can revert the latest changes and propose them separately.

markshannon
markshannon reviewed Nov 11, 2021
View reviewed changes
Comment thread Python/compile.c Outdated
markshannon
markshannon reviewed Nov 11, 2021
View reviewed changes
Comment thread Python/compile.c Outdated
@markshannon

markshannon commented Nov 11, 2021

Copy link
Copy Markdown
Member

I figured I'd give your jump_thread suggestion a stab, and that sort of led to a refactoring of all jump elimination. I do really like the way the logic here cleans up as a result.

I agree, this does clean things up.

@brandtbucher
brandtbucher merged commit 27b69e6 into python:main Nov 11, 2021
@miss-islington

miss-islington commented Nov 11, 2021

Copy link
Copy Markdown
Contributor

Thanks @brandtbucher for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Nov 11, 2021

Copy link
Copy Markdown
Contributor

Sorry, @brandtbucher, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 27b69e60daa7b191ee6bc76fb6d5fb7d793062ab 3.10

brandtbucher added a commit to brandtbucher/cpython that referenced this pull request Nov 11, 2021
@brandtbucher
[3.10] bpo-45773: Stop "optimizing" certain jump patterns (pythonGH-2…
1a35e06 
…9505).

(cherry picked from commit 27b69e6)

Co-authored-by: Brandt Bucher <brandt@python.org>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Nov 11, 2021
@bedevere-bot

bedevere-bot commented Nov 11, 2021

Copy link
Copy Markdown

GH-29526 is a backport of this pull request to the 3.10 branch.

@brandtbucher
brandtbucher deleted the peephole-hang branch November 11, 2021 19:52
thatbirdguythatuknownot added a commit to thatbirdguythatuknownot/cpython that referenced this pull request Nov 23, 2021
@thatbirdguythatuknownot
bpo-45773: Stop "optimizing" certain jump patterns (pythonGH-29505)
c5ad191
remykarem pushed a commit to remykarem/cpython that referenced this pull request Dec 7, 2021
@brandtbucher @remykarem
bpo-45773: Stop "optimizing" certain jump patterns (pythonGH-29505)
02a31cc
remykarem pushed a commit to remykarem/cpython that referenced this pull request Jan 30, 2022
@brandtbucher @remykarem
bpo-45773: Stop "optimizing" certain jump patterns (pythonGH-29505)
d31875c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon requested changes

Assignees

@brandtbucher brandtbucher

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@brandtbucher @bedevere-bot @markshannon @miss-islington @the-knights-who-say-ni