Skip to content

compile.c: statically resolve swap instructions when possible - #18538

Merged
byroot merged 1 commit into
ruby:masterfrom
byroot:peephole-swap
Aug 27, 2026
Merged

compile.c: statically resolve swap instructions when possible#18538
byroot merged 1 commit into
ruby:masterfrom
byroot:peephole-swap

Conversation

@byroot

@byroot byroot commented Aug 27, 2026

Copy link
Copy Markdown
Member

If the two preceeding instructions are known not to be dependent on the state of the stack, then we can eliminate the swap and inverse the two previous instructions.

This mostly eliminate instruction in two cases:

Object.new
0000 opt_getconstant_path                   <ic:0 Object>             (   1)[Li]
0002 putnil
0003 swap
0004 opt_new                                <calldata!mid:new, argc:0, ARGS_SIMPLE>, 11

And:

{**@a, b: 2}
0000 getinstancevariable                    :@a, <is:0>               (   1)[Li]
0003 putspecialobject                       1
0005 swap
0006 putobject                              :b
0008 putobject                              2
0010 opt_send_without_block                 <calldata!mid:core#hash_merge_ptr, argc:3, ARGS_SIMPLE>

Comment thread compile.c Outdated
Comment on lines +372 to +378
type == BIN(putobject) ||
type == BIN(putspecialobject) ||
type == BIN(putnil) ||
type == BIN(putself) ||
type == BIN(duphash) ||
type == BIN(getinstancevariable) ||
type == BIN(getlocal)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Would be worth analyzing some real world code to see if there's some more common cases.

If the two preceeding instructions are known not to be dependent
on the state of the stack, then we can eliminate the `swap` and inverse
the two previous instructions.

This mostly eliminate instruction in two cases:

```ruby
Object.new
```

```
0000 opt_getconstant_path                   <ic:0 Object>             (   1)[Li]
0002 putnil
0003 swap
0004 opt_new                                <calldata!mid:new, argc:0, ARGS_SIMPLE>, 11
```

And:

```ruby
{**@A, b: 2}
```

```
0000 getinstancevariable                    :@A, <is:0>               (   1)[Li]
0003 putspecialobject                       1
0005 swap
0006 putobject                              :b
0008 putobject                              2
0010 opt_send_without_block                 <calldata!mid:core#hash_merge_ptr, argc:3, ARGS_SIMPLE>
```

Co-Authored-By: John Hawthorn <john@hawthorn.email>
@github-actions github-actions Bot added the jit YJIT, ZJIT, ... label Aug 27, 2026
@byroot
byroot enabled auto-merge (rebase) August 27, 2026 19:03
@byroot
byroot merged commit f4813a3 into ruby:master Aug 27, 2026
130 checks passed
@byroot
byroot deleted the peephole-swap branch August 27, 2026 20:02
ruby/parser_test.rb
ruby/ripper_test.rb
ruby/ruby_parser_test.rb
ruby/parameters_signature_test.rb

@eregon eregon Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This actually found that this PR causes 2 :line events on the second line for

return unless x
Foo.new

See https://bugs.ruby-lang.org/issues/22299

I'm aiming to remove all skips in ruby/prism#4223 so it's clear new skips should not be added but are actual regressions for :line events.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sorry about that, I'll have to dig into tracepoint to see if that's possible to fix, or if I have to revert that optimization.

byroot added a commit to byroot/ruby that referenced this pull request Sep 9, 2026
byroot added a commit to byroot/ruby that referenced this pull request Sep 9, 2026
eregon pushed a commit that referenced this pull request Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jit YJIT, ZJIT, ...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants