diff options
author | Aaron Patterson <[email protected]> | 2025-03-20 09:48:34 -0700 |
---|---|---|
committer | Aaron Patterson <[email protected]> | 2025-03-20 11:49:13 -0700 |
commit | 62cc3464d902ee7a399ec8c38606fdc0ee98f05e () | |
tree | 3add6ae6a80475086c9154d5936ce44c79edc0f5 /compile.c | |
parent | f07af59a2f10beeb99878c967cefed2912310030 (diff) |
Remove leading `nop` from block when we don't need it
Blocks insert a leading `nop` instruction in order to execute a "block call" tracepoint. Block compilation unconditionally inserts a leading `nop` plus a label after the instruction: https://.com/ruby/ruby/blob/641f15b1c6bd8921407a1f045573d3b0605f00d3/prism_compile.c#L6867-L6869 This `nop` instruction is used entirely for firing the block entry tracepoint. The label exists so that the block can contain a loop but the block entry tracepoint is executed only once. For example, the following code is an infinite loop, but should only execute the b_call tracepoint once: ```ruby -> { redo }.call ``` Previous to this commit, we would eliminate the `nop` instruction, but only if there were no other jump instructions inside the block. This means that the following code would still contain a leading `nop` even though the label following the `nop` is unused: ```ruby -> { nil if bar } ``` ``` == disasm: #<ISeq:block in <main>@test.rb:1 (1,2)-(1,17)> (catch: FALSE) 0000 nop ( 1)[Bc] 0001 putself [Li] 0002 opt_send_without_block <calldata!mid:bar, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0004 branchunless 8 0006 putnil 0007 leave [Br] 0008 putnil 0009 leave [Br] ``` This commit checks to see if the label inserted after the `nop` is actually a jump target. If it's not a jump target, then we should be safe to eliminate the leading `nop`: ``` > build-master/miniruby --dump=insns test.rb == disasm: #<ISeq:<main>@test.rb:1 (1,0)-(1,17)> 0000 putspecialobject 1 ( 1)[Li] 0002 send <calldata!mid:lambda, argc:0, FCALL>, block in <main> 0005 leave == disasm: #<ISeq:block in <main>@test.rb:1 (1,2)-(1,17)> 0000 putself ( 1)[LiBc] 0001 opt_send_without_block <calldata!mid:bar, argc:0, FCALL|VCALL|ARGS_SIMPLE> 0003 branchunless 7 0005 putnil 0006 leave [Br] 0007 putnil 0008 leave [Br] ``` We have a test for b_call tracepoints that use `redo` here: https://.com/ruby/ruby/blob/aebf96f371c8d874398e0041b798892e545fa206/test/ruby/test_settracefunc.rb#L1728-L1736
Notes: Merged: https://.com/ruby/ruby/pull/12957
-rw-r--r-- | compile.c | 31 |
1 files changed, 29 insertions, 2 deletions
@@ -4367,9 +4367,18 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor) list = FIRST_ELEMENT(anchor); int do_block_optimization = 0; - if (ISEQ_BODY(iseq)->type == ISEQ_TYPE_BLOCK && !ISEQ_COMPILE_DATA(iseq)->catch_except_p) { do_block_optimization = 1; } while (list) { @@ -4386,9 +4395,27 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor) if (do_block_optimization) { INSN * item = (INSN *)list; - if (IS_INSN_ID(item, jump)) { do_block_optimization = 0; } } } if (IS_LABEL(list)) { |