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.

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

Remove superinstructions #105229

Closed
markshannon opened this issue Jun 2, 2023 · 13 comments
Closed

Remove superinstructions #105229

markshannon opened this issue Jun 2, 2023 · 13 comments
Labels
3.13 bugs and security fixes performance Performance or resource usage

Comments

@markshannon
Copy link
Member

markshannon commented Jun 2, 2023

Superinstructions offer a small speedup, but their existence complicates things; mainly instrumentation, and specialization.
The tier2 optimizer is likely to be simpler without superinstructions.

This experiment shows that combining the most common superinstructions into single instructions can outperform superinstructions, rendering superinstructions pointless.

We have already removed almost all superinstructions in specialization. We should remove them altogether.

Linked PRs

@markshannon markshannon added the performance Performance or resource usage label Jun 2, 2023
@markshannon
Copy link
Member Author

faster-cpython/ideas#585

@corona10
Copy link
Member

corona10 commented Jun 2, 2023

Question: One good thing of super instruction is that we can hide the optimization detail to end user.
But from this change, if we need to optmize opcodes with concatnation. Do we have to introduce the new opcode or still prefer to experiment it as the super instruction?

cc @carljm @Fidget-Spinner

@markshannon
Copy link
Member Author

What do you mean by the "end user"?
Python programmers really shouldn't be concerned about bytecode, except in a "I wonder how this works" kind of way.

LOAD_FAST_LOAD_FAST is lot easier to understand than instructions like RETURN_GENERATOR or CLEANUP_THROW

@corona10
Copy link
Member

corona10 commented Jun 2, 2023

What do you mean by the "end user"?

Ah I meant Python programmers. Thanks.

Python programmers really shouldn't be concerned about bytecode, except in a "I wonder how this works" kind of way.

Okay, Looks like it was just my thought. I loved super instruction from the view of hidden optimization implementation detail. (we don't have to update magic number for the optimization)

So still questions left if we decide to remove the current super-instruction mechanism.

  1. What are the criteria of the new opcode that can be added? Can we add any super instructions that can be faster than the current instruction sequences since it doesn't require runtime overhead?
  2. What if the super instruction interferes with general bytecode reordering optimization?
    (I am not sure which case will be)

Thank you for answering for all :)

@corona10
Copy link
Member

corona10 commented Jun 2, 2023

What if the super instruction interferes with general bytecode reordering optimization?
(I am not sure which case will be)

While I am reading the code, it looks like happening at the last moment of optimization. So it will not happen?
(Or can it be issued at Tier 2?)

@markshannon
Copy link
Member Author

Forming superinstructions should done after all other optimizations, so won't interfere those.
It could be done in the assembler.

@markshannon
Copy link
Member Author

What are the criteria of the new opcode that can be added? Can we add any super instructions that can be faster than the current instruction sequences since it doesn't require runtime overhead?

For the reasons given above, I want to remove all superinstructions. The benefit will be negligible after merging #105230, and reasoning about instrumentation, etc. is simpler without them.

@corona10
Copy link
Member

corona10 commented Jun 2, 2023

For the reasons given above, I want to remove all superinstructions.

I am asking about the new single opcodes(in #105230, LOAD_FAST_LOAD_FAST / STORE_FAST_LOAD_FAST / STORE_FAST_LOAD_FAST ) that will alter the existing super instruction. :)
Sorry to make you confused!
I am asking about a situation when someone needs to add a new single opcode concatenated by multiple opcodes.
Because it doesn't require a runtime cost, so the execution of frequency is less important than before.

@terryjreedy
Copy link
Member

This experiment shows

Link goes nowhere for me.

@markshannon
Copy link
Member Author

Fixed the link

@gvanrossum
Copy link
Member

gvanrossum commented Jun 9, 2023

@markshannon Should I just remove the super-instruction support from the cases generator? That would simplify a lot of things! (There are a bunch of things that are factored somewhat awkwardly to share common code between super and macro while also honoring their differences.)

DECISION: Yes, remove it. Also remove support for legacy instr syntax (without stack/cache effect syntax).

@gvanrossum
Copy link
Member

@carljm FYI. I forgot that Cinder now uses the generator to override some opcodes and add others. Hopefully you're not using super-instructions, or we'd have to roll back gh-105703.

@carljm
Copy link
Member

carljm commented Jun 12, 2023

@carljm FYI. I forgot that Cinder now uses the generator to override some opcodes and add others. Hopefully you're not using super-instructions, or we'd have to roll back gh-105703.

Nope, we don't need the super-instruction support. Thanks for thinking of us! I am generally watching changes in these areas and will pipe up if I see anything happening that would be a problem for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes performance Performance or resource usage
Projects
None yet
Development

No branches or pull requests

6 participants