Skip to content

Comments

bpo-29469: Remove unnecessary peephole optimizer#4863

Merged
methane merged 1 commit intopython:masterfrom
methane:remove-not-binary
Dec 14, 2017
Merged

bpo-29469: Remove unnecessary peephole optimizer#4863
methane merged 1 commit intopython:masterfrom
methane:remove-not-binary

Conversation

@methane
Copy link
Member

@methane methane commented Dec 14, 2017 •
edited by bedevere-bot
Loading

Conversions like not a is b -> a is not b are implemented
in AST optimizer in 7ea143a.
This commit removes them from peephole optimizer.

https://bugs.python.org/issue29469

@methane
bpo-29469: Remove unnecessary peephole optimizer
19b2fa8 
Conversions like `not a is b -> a is not b` are implemented
in AST optimizer in previous commit.
So this commit removes them from peephole optimizer.
@methane methane mentioned this pull request Dec 14, 2017
Merged
@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Dec 14, 2017

See also Raymond's comment on the tracker.

@methane
Copy link
Member Author

methane commented Dec 14, 2017

I saw it but I don't understand it yet.
Is it easy enough to do in this pull request?

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Dec 14, 2017

Are CONST_STACK_* macros and lastn_const_start() and fold_tuple_on_constants() still needed?

@methane
Copy link
Member Author

methane commented Dec 14, 2017

I'm reading it now.

@methane
Copy link
Member Author

methane commented Dec 14, 2017

I did it, but importlib.h and importlib_external.h has changed.
I'm checking what happened.

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Dec 14, 2017

You have removed the optimization for for i in [1, 2]. It can be implemented at AST level.

@methane
Copy link
Member Author

methane commented Dec 14, 2017

Removing BUILD_TUPLE folding makes regression on MAKE_FUNCTION:

before:

 459         204 LOAD_CONST              96 ((None, None))
             206 LOAD_CONST              39 (<code object _spec_from_module at 0x7f7d61834420, file "_bootstrap.py", line 459>)
             208 LOAD_CONST              40 ('_spec_from_module')
             210 MAKE_FUNCTION            1
             212 STORE_NAME              22 (_spec_from_module)

after:

 459         204 LOAD_CONST               1 (None)
             206 LOAD_CONST               1 (None)
             208 BUILD_TUPLE              2
             210 LOAD_CONST              39 (<code object _spec_from_module at 0x7f5f36a2c5d0, file "_bootstrap.py", line 459>)
             212 LOAD_CONST              40 ('_spec_from_module')
             214 MAKE_FUNCTION            1
             216 STORE_NAME              22 (_spec_from_module)

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Dec 14, 2017

Oh, right. And this can't be moved to the AST level. Thus tuple constants folding should be kept. It can be moved to the compiler lever, but this is a separate complex issue.

@methane
Copy link
Member Author

methane commented Dec 14, 2017

Maybe, I need to re-implement fold_tuple_on_constants() to export consts from
codes. But I have no time for it today.

@methane methane merged commit eadad1b into python:master Dec 14, 2017
@methane methane deleted the remove-not-binary branch December 14, 2017 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@methane @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot