Skip to content

Commit 62cc346

Browse files
committed
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://github.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://github.com/ruby/ruby/blob/aebf96f371c8d874398e0041b798892e545fa206/test/ruby/test_settracefunc.rb#L1728-L1736
1 parent f07af59 commit 62cc346

File tree

2 files changed

+30
-3
lines changed

2 files changed

+30
-3
lines changed

compile.c

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4367,9 +4367,18 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
43674367
list = FIRST_ELEMENT(anchor);
43684368

43694369
int do_block_optimization = 0;
4370+
LABEL * block_loop_label = NULL;
43704371

4371-
if (ISEQ_BODY(iseq)->type == ISEQ_TYPE_BLOCK && !ISEQ_COMPILE_DATA(iseq)->catch_except_p) {
4372+
// If we're optimizing a block
4373+
if (ISEQ_BODY(iseq)->type == ISEQ_TYPE_BLOCK) {
43724374
do_block_optimization = 1;
4375+
4376+
// If the block starts with a nop and a label,
4377+
// record the label so we can detect if it's a jump target
4378+
LINK_ELEMENT * le = FIRST_ELEMENT(anchor)->next;
4379+
if (IS_INSN(le) && IS_INSN_ID((INSN *)le, nop) && IS_LABEL(le->next)) {
4380+
block_loop_label = (LABEL *)le->next;
4381+
}
43734382
}
43744383

43754384
while (list) {
@@ -4386,9 +4395,27 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
43864395

43874396
if (do_block_optimization) {
43884397
INSN * item = (INSN *)list;
4389-
if (IS_INSN_ID(item, jump)) {
4398+
// Give up if there is a throw
4399+
if (IS_INSN_ID(item, throw)) {
43904400
do_block_optimization = 0;
43914401
}
4402+
else {
4403+
// If the instruction has a jump target, check if the
4404+
// jump target is the block loop label
4405+
const char *types = insn_op_types(item->insn_id);
4406+
for (int j = 0; types[j]; j++) {
4407+
if (types[j] == TS_OFFSET) {
4408+
// If the jump target is equal to the block loop
4409+
// label, then we can't do the optimization because
4410+
// the leading `nop` instruction fires the block
4411+
// entry tracepoint
4412+
LABEL * target = (LABEL *)OPERAND_AT(item, j);
4413+
if (target == block_loop_label) {
4414+
do_block_optimization = 0;
4415+
}
4416+
}
4417+
}
4418+
}
43924419
}
43934420
}
43944421
if (IS_LABEL(list)) {

test/ruby/test_yjit.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,7 @@ def +(x) = self - -x
13411341
end
13421342

13431343
def test_tracing_str_uplus
1344-
assert_compiles(<<~RUBY, frozen_string_literal: true, result: :ok, exits: { putspecialobject: 1, definemethod: 1 })
1344+
assert_compiles(<<~RUBY, frozen_string_literal: true, result: :ok, exits: { putspecialobject: 1 })
13451345
def str_uplus
13461346
_ = 1
13471347
_ = 2

0 commit comments

Comments
 (0)