Skip to content

bpo-45843: Optimize constant comparisons/contains #29639

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

Closed

Conversation

thatbirdguythatuknownot
Copy link
Contributor

@thatbirdguythatuknownot thatbirdguythatuknownot commented Nov 19, 2021

@Fidget-Spinner
Copy link
Member

Thanks for the really impressive compiler work.

I hope I understand the code correctly. It's effectively doing constant folding for comparison expressions where both sides are consts right? So 1<2 goes from

LOAD_CONST 1
LOAD_CONST 2
COMPARE_OP 0

to

LOAD_CONST 0 (True)

My question is how common do people write things like 1<2 in production code? Constant folding for things like 10*64 make sense for readability, but I don't see how 1<2 is useful.

@markshannon
Copy link
Member

I was surprised that this isn't handled by the AST optimizer.
In fact there is "to do" for precisely this:
https://github.com/python/cpython/blob/main/Python/ast_opt.c#L621

@thatbirdguythatuknownot could you implement this in the AST optimizer?

@markshannon
Copy link
Member

I suspect that no one would write if 1 < 2:, but there is an expectation that Python will fold constants and that the body of

if 0:
     sensitive_internal_debugging_code

is removed from the bytecode.

So this is probably worth doing from a consistency point of view, even if it makes no difference in terms of performance.

@thatbirdguythatuknownot
Copy link
Contributor Author

thatbirdguythatuknownot commented Nov 22, 2021

I suspect that no one would write if 1 < 2:, but there is an expectation that Python will fold constants and that the body of

if 0:
     sensitive_internal_debugging_code

is removed from the bytecode.

So this is probably worth doing from a consistency point of view, even if it makes no difference in terms of performance.

@markshannon Should I revert the changes made to Python/compiler.c? I have made changes to Python/ast_opt.c.

@thatbirdguythatuknownot thatbirdguythatuknownot changed the title bpo-45843: Optimize LOAD_CONST followed by COMPARE_OP or IS_OP bpo-45843: Optimize LOAD_CONST followed by comparison/contain operators Nov 22, 2021
@thatbirdguythatuknownot thatbirdguythatuknownot changed the title bpo-45843: Optimize LOAD_CONST followed by comparison/contain operators bpo-45843: Optimize constant comparisons/contains Nov 22, 2021
@markshannon
Copy link
Member

I don't understand why you need all the extra code.

All you need to do in fold_compare is check that both sides of the comparisons are constants and perform the comparison.
See fold_subscr for an example.

@thatbirdguythatuknownot
Copy link
Contributor Author

thatbirdguythatuknownot commented Nov 22, 2021

I don't understand why you need all the extra code.

All you need to do in fold_compare is check that both sides of the comparisons are constants and perform the comparison. See fold_subscr for an example.

@markshannon Do I return if there is a comparison that does not have a constant? Otherwise, I'm handling it with the rest of the constant comparisons.

@isidentical
Copy link
Member

Closing since this issue got rejected on the bug tracker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants