Skip to content

YJIT: Support invokeblock #6640

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

Merged
merged 3 commits into from
Nov 2, 2022
Merged

YJIT: Support invokeblock #6640

merged 3 commits into from
Nov 2, 2022

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Oct 27, 2022

This PR implements:

  • YJIT codegen to yield an ISEQ
  • Other changes:
    • Add another register to get_alloc_regs on x86_64.
      • In today's pairing with Maxime and Alan, we concluded that it's helpful to have the third register for this case. If we have only two registers, gen_get_lep will need to be called again and again to split the live range, but it's not efficient. Besides, using the same number of registers for Intel and Arm would make our life easier; we often need to fix a new issue when we work on stuff on Arm and then switch to Intel with fewer registers.
    • Improve split_bitmask_immediate with a negative immediate on Arm: less asm.load / register spill pressure.
    • Rename rb_get_iseq_flags_has_accepts_no_kwarg to rb_get_iseq_flags_accepts_no_kwarg: The naming seems inconsistent with rb_get_iseq_flags_ruby2_keywords having no has_.

This results in fewer side exists on railsbench:

Before

ratio_in_yjit:              89.7%

Top-20 most frequent exit ops (100.0% of exits):
    opt_send_without_block:     309533 (45.8%)
               invokesuper:     116375 (17.2%)
                      send:     102990 (15.2%)
               invokeblock:      82721 (12.2%)
               getconstant:      17336 (2.6%)

After

ratio_in_yjit:              90.2%

Top-20 most frequent exit ops (100.0% of exits):
    opt_send_without_block:     309575 (49.1%)
               invokesuper:     116399 (18.5%)
                      send:     102994 (16.3%)
               invokeblock:      36605 (5.8%)
               getconstant:      17336 (2.7%)

The following exit reasons are not addressed in this PR as they don't seem like a bottleneck:

invokeblock exit reasons:
               ifunc       5765 (57.8%)
                proc       2073 (20.8%)
     iseq_arg0_splat       2000 (20.1%)
    iseq_tag_changed         78 ( 0.8%)
              symbol         52 ( 0.5%)

@k0kubun k0kubun force-pushed the yjit-invokeblock branch 12 times, most recently from 3de640f to d85d6d4 Compare November 1, 2022 21:21
@k0kubun k0kubun marked this pull request as ready for review November 1, 2022 22:51
@matzbot matzbot requested a review from a team November 1, 2022 22:51
};
asm.store(Opnd::mem(64, sp, SIZEOF_VALUE_I32 * -2), specval);

// Arm requires another register to load the immediate value of Qnil before storing it.
// So donig this after releasing the register for specval to avoid register spill.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/donig/doing/

@noahgibbs
Copy link
Contributor

LGTM. At some point I should see if I can figure out approximately how much mem we're using for the context structures. I don't think the new captured-self entry should be a big increase in mem size... But I also haven't tried to figure out how many context structs we allocate, or whether we can easily tell from our stats.

Copy link
Contributor

@maximecb maximecb left a comment

Choose a reason for hiding this comment

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

Well done :)

@maximecb maximecb merged commit 81e84e0 into ruby:master Nov 2, 2022
@maximecb maximecb deleted the yjit-invokeblock branch November 2, 2022 16:30
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.

3 participants