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

gh-90997: bpo-46841: Disassembly of quickened code #32099

Merged
merged 16 commits into from
Apr 19, 2022

Conversation

penguin-wwy
Copy link
Contributor

@penguin-wwy penguin-wwy commented Mar 24, 2022

@brandtbucher
Copy link
Member

Is this ready for review?

If not, perhaps mark it as a "draft" for now.

@brandtbucher brandtbucher self-requested a review March 24, 2022 17:12
@penguin-wwy penguin-wwy marked this pull request as draft March 24, 2022 18:03
@penguin-wwy penguin-wwy marked this pull request as ready for review March 25, 2022 14:43
Lib/opcode.py Outdated Show resolved Hide resolved
@markshannon
Copy link
Member

Looks promising. Will need some tests for the new output format.

@penguin-wwy
Copy link
Contributor Author

One problem is that some instructions are too long(like PRECALL_BUILTIN_FAST_WITH_KEYWORDS), do I need to increase _OPNAME_WIDTH

def adaptive_test(a, b):
    c = a + b
    print(c.__class__)

Exec dis(adaptive_test):

 30           0 RESUME                   0

 31           2 LOAD_FAST                0 (a)
              4 LOAD_FAST                1 (b)
              6 BINARY_OP                0 (+)
             10 STORE_FAST               2 (c)

 32          12 LOAD_GLOBAL              1 (NULL + print)
             24 LOAD_FAST                2 (c)
             26 LOAD_ATTR                1 (__class__)
             36 PRECALL                  1
             40 CALL                     1
             50 POP_TOP
             52 LOAD_CONST               0 (None)
             54 RETURN_VALUE

Exec dis(adaptive_test, adaptive=True):

 30           0 RESUME_QUICK             0

 31           2 LOAD_FAST__LOAD_FAST     0 (a)
              4 LOAD_FAST                1 (b)
              6 BINARY_OP_ADD_INT        0 (+)
             10 STORE_FAST               2 (c)

 32          12 LOAD_GLOBAL_BUILTIN      1 (NULL + print)
             24 LOAD_FAST                2 (c)
             26 LOAD_ATTR_SLOT           1 (__class__)
             36 PRECALL_BUILTIN_FAST_WITH_KEYWORDS     1
             40 CALL_ADAPTIVE            1
             50 POP_TOP
             52 LOAD_CONST               0 (None)
             54 RETURN_VALUE

@penguin-wwy
Copy link
Contributor Author

penguin-wwy commented Apr 2, 2022

Looks promising. Will need some tests for the new output format.

Already add some test cases for quicken code in the dis.

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

Could you remove the changes to _testinternalcapi, so we don't need to export the _PyCode_Quicken function.

Lib/test/test_dis.py Outdated Show resolved Hide resolved
@penguin-wwy penguin-wwy changed the title bpo-46841: Disassembly of quickened code gh-90997: bpo-46841: Disassembly of quickened code Apr 14, 2022
@penguin-wwy
Copy link
Contributor Author

I have made the requested changes; please review again :) @markshannon @brandtbucher

@bedevere-bot
Copy link

Thanks for making the requested changes!

: please review the changes made to this pull request.

Lib/test/test_dis.py Outdated Show resolved Hide resolved
@markshannon
Copy link
Member

On small typo, otherwise looks good.

@penguin-wwy
Copy link
Contributor Author

Thanks. :)
I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

: please review the changes made to this pull request.

@markshannon
Copy link
Member

Thanks @penguin-wwy for doing this.

Sign up for free to 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