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-106529: Implement POP_JUMP_IF_XXX uops #106551

Merged
merged 8 commits into from
Jul 10, 2023
Merged

Conversation

gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Jul 8, 2023

This adds:

  • Hand-written uops _POP_JUMP_IF_{TRUE,FALSE}. These pop the top of the stack. The jump target (in superblock space) is absolute.

  • Hand-written translation for POP_JUMP_IF_{TRUE,FALSE}, assuming the jump is unlikely. Once we implement jump-likelihood profiling, we can implement the jump-unlikely case.

Note that the stubs are placed at the end of the trace array (and not counted in the return value of translate_bytecode_to_trace(), which is a bid dodgy). There is a gap between the main superblock and the stubs. We could close the gap, we'd just have to patch up the JUMP uops.

TODO in this PR:

  • The other POP_JUMP_IF_XXX instructions
  • Close the gap, and return trace size including stubs

TODO in a follow-up PR:

  • Profile branch likelihood
  • Use that in translating the branches

This adds:

- Hand-written uops JUMP_IF_{TRUE,FALSE,NONE,NOT_NONE}.
  These peek at the top of the stack.
  The jump target (in superblock space) is absolute.

- Hand-written translation for POP_JUMP_IF_TRUE,
  assuming the jump is unlikely.
  This can serve as an example for the rest.
  (Probably adding the others will cause a little refactoring.)
  Once we implement jump-likelihood profiling,
  we can implement the jump-unlikely case.

Note that the stubs are placed at the end of the trace array.
There is a gap between the main superblock and the stubs.
We could close the gap, we'd just have to patch up the JUMP uops.
@gvanrossum gvanrossum marked this pull request as ready for review July 10, 2023 18:29
@gvanrossum gvanrossum requested a review from markshannon as a code owner July 10, 2023 18:29
@gvanrossum
Copy link
Member Author

Since we discussed this extensively off-line, I'm going to just merge this once all tests pass.

This required improving len(ex) and ex[i] a bit.
There is now a sentinel opcode (0) at the end,
*unless* the trace array is full.
@gvanrossum gvanrossum merged commit 22988c3 into python:main Jul 10, 2023
@gvanrossum gvanrossum deleted the jump-if-uops branch July 10, 2023 23:04
gvanrossum added a commit that referenced this pull request Jul 11, 2023
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.

2 participants