Skip to content

gh-93554: Conditional jumps only jump forward - #96318

Merged
iritkatriel merged 11 commits into
python:mainfrom
iritkatriel:conditional_jump_only_forward
Sep 1, 2022
Merged

gh-93554: Conditional jumps only jump forward#96318
iritkatriel merged 11 commits into
python:mainfrom
iritkatriel:conditional_jump_only_forward

Conversation

@iritkatriel

@iritkatrieliritkatriel commented Aug 26, 2022

Copy link
Copy Markdown
Member

@iritkatrieliritkatriel added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 26, 2022
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 8a3d017 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 26, 2022
@iritkatriel
iritkatriel marked this pull request as ready for review August 26, 2022 18:15
@iritkatrieliritkatriel self-assigned this Aug 26, 2022
Comment threadDoc/library/dis.rst Outdated
Comment threadDoc/library/dis.rst Outdated
Comment threadDoc/library/dis.rst Outdated
Comment threadPython/compile.c

@brandtbucherbrandtbucher 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.

Thanks for doing this!

Just curious, have you benchmarked this change?

@brandtbucher

brandtbucher commented Aug 30, 2022

Copy link
Copy Markdown
Member

We might also consider adding a test that loops ending in branches can still quicken code.

For example, something like:

deff(x: bool) ->None:
foriinrange(1_000_000):
ifx:
pass

Today f(True) quickens the code, but f(False) doesn't. After this change, both should.

Ditto for while loops. This code should be quickened after only one call:

defw() ->None:
i=0whilei<1_000_000:
i+=1

@iritkatriel

Copy link
Copy Markdown
MemberAuthor

Just curious, have you benchmarked this change?

I haven't yet.

@markshannon

markshannon commented Aug 31, 2022

Copy link
Copy Markdown
Member

Small speedup

@iritkatriel

Copy link
Copy Markdown
MemberAuthor

We might also consider adding a test that loops ending in branches can still quicken code.

For example, something like:

deff(x: bool) ->None:
foriinrange(1_000_000):
ifx:
pass

Today f(True) quickens the code, but f(False) doesn't. After this change, both should.

Ditto for while loops. This code should be quickened after only one call:

defw() ->None:
i=0whilei<1_000_000:
i+=1

Where does such a test belong?

@iritkatrieliritkatriel added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 1, 2022
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit e87887c 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 1, 2022
@iritkatriel

Copy link
Copy Markdown
MemberAuthor

I'll merge this while the buildbots are happy with it, we can add more tests (for specialisation etc) in a followup PR.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@iritkatriel@bedevere-bot@brandtbucher@markshannon@sweeneyde