From 62cc3464d902ee7a399ec8c38606fdc0ee98f05e Mon Sep 17 00:00:00 2001
From: Aaron Patterson <tenderlove@ruby-lang.org>
Date: Thu, 20 Mar 2025 09:48:34 -0700
Subject: [PATCH] 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
---
 compile.c              | 31 +++++++++++++++++++++++++++++--
 test/ruby/test_yjit.rb |  2 +-
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/compile.c b/compile.c
index 7aac2f9dbc..cd7e92298c 100644
--- a/compile.c
+++ b/compile.c
@@ -4367,9 +4367,18 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
     list = FIRST_ELEMENT(anchor);
 
     int do_block_optimization = 0;
+    LABEL * block_loop_label = NULL;
 
-    if (ISEQ_BODY(iseq)->type == ISEQ_TYPE_BLOCK && !ISEQ_COMPILE_DATA(iseq)->catch_except_p) {
+    // If we're optimizing a block
+    if (ISEQ_BODY(iseq)->type == ISEQ_TYPE_BLOCK) {
         do_block_optimization = 1;
+
+        // If the block starts with a nop and a label,
+        // record the label so we can detect if it's a jump target
+        LINK_ELEMENT * le = FIRST_ELEMENT(anchor)->next;
+        if (IS_INSN(le) && IS_INSN_ID((INSN *)le, nop) && IS_LABEL(le->next)) {
+            block_loop_label = (LABEL *)le->next;
+        }
     }
 
     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)) {
+                // Give up if there is a throw
+                if (IS_INSN_ID(item, throw)) {
                     do_block_optimization = 0;
                 }
+                else {
+                    // If the instruction has a jump target, check if the
+                    // jump target is the block loop label
+                    const char *types = insn_op_types(item->insn_id);
+                    for (int j = 0; types[j]; j++) {
+                        if (types[j] == TS_OFFSET) {
+                            // If the jump target is equal to the block loop
+                            // label, then we can't do the optimization because
+                            // the leading `nop` instruction fires the block
+                            // entry tracepoint
+                            LABEL * target = (LABEL *)OPERAND_AT(item, j);
+                            if (target == block_loop_label) {
+                                do_block_optimization = 0;
+                            }
+                        }
+                    }
+                }
             }
         }
         if (IS_LABEL(list)) {
diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb
index 450e6e5625..7c0524354b 100644
--- a/test/ruby/test_yjit.rb
+++ b/test/ruby/test_yjit.rb
@@ -1341,7 +1341,7 @@ class TestYJIT < Test::Unit::TestCase
   end
 
   def test_tracing_str_uplus
-    assert_compiles(<<~RUBY, frozen_string_literal: true, result: :ok, exits: { putspecialobject: 1, definemethod: 1 })
+    assert_compiles(<<~RUBY, frozen_string_literal: true, result: :ok, exits: { putspecialobject: 1 })
       def str_uplus
         _ = 1
         _ = 2