0001-send-pop-optimisation-part-two-eliminate-pop.patch

From 366e7e37b5f54c5d359a32f4324b6b90a6c7d33a Mon Sep 17 00:00:00 2001
From: "Urabe, Shyouhei" <shyouhei@ruby-lang.org>
Date: Mon, 21 Jan 2019 19:04:17 +0900
Subject: [PATCH] send-pop optimisation part two: eliminate pop

Sending a method, then immediately throwing away its return value, is
one of the most frequent waste of time that ruby does.  Now that
callee methods can skip pushing objects onto the stack, why not caller
sites to also avoid popping them.

In order to do so our compiler now does not emit pop instructions but
add VM_CALL_POPPED flag to the call info of adjacent send-ish
instructions.  It is now the caller's duty to properly igonre the
return value.

Signed-off-by: Urabe, Shyouhei <shyouhei@ruby-lang.org>
---
 compile.c                                   | 90 +++++++++++++++++++--
 insns.def                                   | 46 ++++++-----
 iseq.c                                      |  1 +
 tool/ruby_vm/helpers/dumper.rb              |  7 ++
 tool/ruby_vm/models/bare_instructions.rb    |  1 -
 tool/ruby_vm/models/instructions.rb         |  1 +
 tool/ruby_vm/models/sendpop_instructions.rb | 41 ++++++++++
 tool/ruby_vm/views/_insn_entry.erb          |  3 +
 tool/ruby_vm/views/_sp_inc_helpers.erb      | 15 ++--
 tool/ruby_vm/views/vm.inc.erb               | 13 +++
 vm_core.h                                   |  7 +-
 vm_dump.c                                   |  6 ++
 vm_insnhelper.c                             | 12 +--
 vm_insnhelper.h                             | 13 +++
 14 files changed, 214 insertions(+), 42 deletions(-)
 create mode 100644 tool/ruby_vm/models/sendpop_instructions.rb

diff --git a/compile.c b/compile.c
index 9cd2ff21f9..b6dba621d8 100644
--- a/compile.c
+++ b/compile.c
@@ -3286,7 +3286,10 @@ iseq_specialized_instruction(rb_iseq_t *iseq, INSN *iobj)
 	struct rb_call_info *ci = (struct rb_call_info *)OPERAND_AT(iobj, 0);
 	const rb_iseq_t *blockiseq = (rb_iseq_t *)OPERAND_AT(iobj, 2);
 
-#define SP_INSN(opt) insn_set_specialized_instruction(iseq, iobj, BIN(opt_##opt))
+#define SP_INSN(opt) insn_set_specialized_instruction(iseq, iobj, \
+            (ci->flag & VM_CALL_POPPED) \
+            ? BIN(opt_sendpop_opt_##opt) \
+            : BIN(opt_##opt))
 	if (ci->flag & VM_CALL_ARGS_SIMPLE) {
 	    switch (ci->orig_argc) {
 	      case 0:
@@ -3330,9 +3333,14 @@ iseq_specialized_instruction(rb_iseq_t *iseq, INSN *iobj)
 	}
 
 	if ((ci->flag & VM_CALL_ARGS_BLOCKARG) == 0 && blockiseq == NULL) {
-	    iobj->insn_id = BIN(opt_send_without_block);
+            iobj->insn_id = (ci->flag & VM_CALL_POPPED)
+                ? BIN(opt_sendpop_opt_send_without_block)
+                : BIN(opt_send_without_block);
 	    iobj->operand_size = insn_len(iobj->insn_id) - 1;
 	}
+        else if (ci->flag & VM_CALL_POPPED) {
+            iobj->insn_id = BIN(opt_sendpop_send);
+        }
     }
 #undef SP_INSN
 
@@ -3356,17 +3364,17 @@ tailcallable_p(rb_iseq_t *iseq)
     }
 }
 
-static bool
+static struct rb_call_info *
 is_the_insn_sendish(const INSN *i)
 {
     const char *t = insn_op_types(INSN_OF(i));
 
     for (int j = 0; t[j]; j++) {
         if (t[j] == TS_CALLINFO) {
-            return true;
+            return (void *)OPERAND_AT(i, j);
         }
     }
-    return false;
+    return NULL;
 }
 
 static enum rb_insn_purity
@@ -3536,6 +3544,75 @@ iseq_insert_bailouts(rb_iseq_t *iseq, LINK_ANCHOR *anchor)
     }
 }
 
+static void
+iseq_sendpop_optimization_phase1(
+    const rb_iseq_t * iseq,
+    const LINK_ANCHOR * anchor)
+{
+    for (const LINK_ELEMENT *e = FIRST_ELEMENT(anchor); e->next; e = e->next) {
+        struct rb_call_info *ci;
+
+        if (! IS_INSN(e)) {
+            continue;
+        }
+        else if (! IS_INSN(e->next)) {
+            continue;
+        }
+        else if (! (ci = is_the_insn_sendish((const INSN *)e))) {
+            continue;
+        }
+        else if (! IS_INSN_ID(e->next, pop)) {
+            /* We are going to optimize only if send and pop are
+             * direct adjacent i.e. no jump labels are between.  If
+             * the pop is a jump destination that shall not be
+             * optimized out. */
+            continue;
+        }
+        else if (IS_INSN_ID(e, invokeblock)) {
+            /* We cannot optimize invokeblock.  Lambdas migth tag jump
+             * using TAG_RETURN. If we support such case, we have to
+             * store the "continuation" somewhere, every time, even
+             * when the lambda actually do not tag jump, to indicate
+             * we need to pop the TAG_RETURN return value.  Just
+             * pushing the return value on top of the stack is way
+             * lightweight than that. */
+            continue;
+        }
+        else {
+            ci->flag |= VM_CALL_POPPED;
+        }
+    }
+}
+
+static void
+iseq_sendpop_optimization_phase2(
+    const rb_iseq_t * iseq,
+    LINK_ANCHOR * anchor)
+{
+    for (LINK_ELEMENT *e = FIRST_ELEMENT(anchor); e->next; e = e->next) {
+        const struct rb_call_info *ci;
+
+        if (! IS_INSN(e)) {
+            continue;
+        }
+        else if (! IS_INSN(e->next)) {
+            continue;
+        }
+        else if (! (ci = is_the_insn_sendish((const INSN *)e))) {
+            continue;
+        }
+        else if (! IS_INSN_ID(e->next, pop)) {
+            continue;
+        }
+        else if (! (ci->flag & VM_CALL_POPPED)) {
+            continue;
+        }
+        else {
+            ELEM_REMOVE(e->next);
+        }
+    }
+}
+
 static int
 iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
 {
@@ -3549,6 +3626,7 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
     int tailcallopt = do_tailcallopt;
 
     list = FIRST_ELEMENT(anchor);
+    iseq_sendpop_optimization_phase1(iseq, anchor);
 
     while (list) {
 	if (IS_INSN(list)) {
@@ -3575,6 +3653,8 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
 	}
 	list = list->next;
     }
+
+    iseq_sendpop_optimization_phase2(iseq, anchor);
     iseq_insert_bailouts(iseq, anchor);
     return COMPILE_OK;
 }
diff --git a/insns.def b/insns.def
index 246c9ab9b5..98dd3c251a 100644
--- a/insns.def
+++ b/insns.def
@@ -768,15 +768,14 @@ send
 // attr rb_snum_t sp_inc = sp_inc_of_sendish(ci);
 {
     VALUE bh = vm_caller_setup_arg_block(ec, GET_CFP(), ci, blockiseq, false);
-    int popped =
-        CURRENT_INSN_IS(pop)         ? VM_FRAME_FLAG_POPPED :
-        CURRENT_INSN_IS(leave)       ? VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) :
-        CURRENT_INSN_IS(opt_bailout) ? VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) :
-        0;
+    int popped = INSN_CALLER_RETVAL_POPPED_P();
     val = vm_sendish(ec, GET_CFP(), ci, cc, bh, popped, vm_search_method_wrap);
 
     if (val == Qundef) {
         RESTORE_REGS();
+#if INSN_CLASS == RUBYVM_SENDPOP_INSTRUCTION
+        VM_ENV_FLAGS_SET(GET_EP(), VM_FRAME_FLAG_POPIT);
+#endif
         NEXT_INSN();
     }
 }
@@ -792,15 +791,14 @@ opt_send_without_block
 // attr rb_snum_t sp_inc = sp_inc_of_sendish(ci);
 {
     VALUE bh = VM_BLOCK_HANDLER_NONE;
-    int popped =
-        CURRENT_INSN_IS(pop)         ? VM_FRAME_FLAG_POPPED :
-        CURRENT_INSN_IS(leave)       ? VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) :
-        CURRENT_INSN_IS(opt_bailout) ? VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) :
-        0;
+    int popped = INSN_CALLER_RETVAL_POPPED_P();
     val = vm_sendish(ec, GET_CFP(), ci, cc, bh, popped, vm_search_method_wrap);
 
     if (val == Qundef) {
         RESTORE_REGS();
+#if INSN_CLASS == RUBYVM_SENDPOP_INSTRUCTION
+        VM_ENV_FLAGS_SET(GET_EP(), VM_FRAME_FLAG_POPIT);
+#endif
         NEXT_INSN();
     }
 }
@@ -872,15 +870,14 @@ invokesuper
 // attr rb_snum_t sp_inc = sp_inc_of_sendish(ci);
 {
     VALUE bh = vm_caller_setup_arg_block(ec, GET_CFP(), ci, blockiseq, true);
-    int popped =
-        CURRENT_INSN_IS(pop)         ? VM_FRAME_FLAG_POPPED :
-        CURRENT_INSN_IS(leave)       ? VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) :
-        CURRENT_INSN_IS(opt_bailout) ? VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) :
-        0;
+    int popped = INSN_CALLER_RETVAL_POPPED_P();
     val = vm_sendish(ec, GET_CFP(), ci, cc, bh, popped, vm_search_super_method);
 
     if (val == Qundef) {
         RESTORE_REGS();
+#if INSN_CLASS == RUBYVM_SENDPOP_INSTRUCTION
+        VM_ENV_FLAGS_SET(GET_EP(), VM_FRAME_FLAG_POPIT);
+#endif
         NEXT_INSN();
     }
 }
@@ -901,15 +898,14 @@ invokeblock
 
 
     VALUE bh = VM_BLOCK_HANDLER_NONE;
-    int popped =
-        CURRENT_INSN_IS(pop)         ? VM_FRAME_FLAG_POPPED :
-        CURRENT_INSN_IS(leave)       ? VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) :
-        CURRENT_INSN_IS(opt_bailout) ? VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) :
-        0;
+    int popped = INSN_CALLER_RETVAL_POPPED_P();
     val = vm_sendish(ec, GET_CFP(), ci, &cc, bh, popped, vm_search_invokeblock);
 
     if (val == Qundef) {
         RESTORE_REGS();
+#if INSN_CLASS == RUBYVM_SENDPOP_INSTRUCTION
+        VM_ENV_FLAGS_SET(GET_EP(), VM_FRAME_FLAG_POPIT);
+#endif
         NEXT_INSN();
     }
 }
@@ -919,7 +915,7 @@ DEFINE_INSN
 leave
 ()
 (VALUE val)
-(VALUE val)
+(...)
 /* This is super surprising but when leaving from a frame, we check
  * for interrupts.  If any, that should be executed on top of the
  * current execution context.  This is a method call. */
@@ -931,9 +927,12 @@ leave
 // attr enum rb_insn_purity purity = rb_insn_is_pure;
 /* And this instruction handles SP by nature. */
 // attr bool handles_sp = true;
+// attr rb_snum_t sp_inc = 0;
 {
+    bool popit = VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPIT);
+
     if (OPT_CHECKED_RUN) {
-	const VALUE *const bp = vm_base_ptr(reg_cfp);
+        const VALUE *const bp = vm_base_ptr(reg_cfp) + (popit ? 1 : 0);
 	if (reg_cfp->sp != bp) {
 	    vm_stack_consistency_error(ec, reg_cfp, bp);
 	}
@@ -951,6 +950,9 @@ leave
     }
     else {
 	RESTORE_REGS();
+        if (! popit) {
+            PUSH(val);
+        }
     }
 }
 
diff --git a/iseq.c b/iseq.c
index 486dbdaf41..faad35df81 100644
--- a/iseq.c
+++ b/iseq.c
@@ -1864,6 +1864,7 @@ rb_insn_operand_intern(const rb_iseq_t *iseq,
 		CALL_FLAG(KWARG);
 		CALL_FLAG(KW_SPLAT);
 		CALL_FLAG(OPT_SEND); /* maybe not reachable */
+		CALL_FLAG(POPPED);
 		rb_ary_push(ary, rb_ary_join(flags, rb_str_new2("|")));
 	    }
 	    ret = rb_sprintf("<callinfo!%"PRIsVALUE">", rb_ary_join(ary, rb_str_new2(", ")));
diff --git a/tool/ruby_vm/helpers/dumper.rb b/tool/ruby_vm/helpers/dumper.rb
index c0a45b19e2..d5dd531981 100644
--- a/tool/ruby_vm/helpers/dumper.rb
+++ b/tool/ruby_vm/helpers/dumper.rb
@@ -109,4 +109,11 @@ def generate template
   def render_c_expr expr
     render 'c_expr', locals: { expr: expr, }
   end
+
+  def upcase str
+    # Should stick to ASCII
+    # upcase(:ascii) is not available for old ruby
+    cstr = as_tr_cpp str
+    cstr.tr 'a-z', 'A-Z'
+  end
 end
diff --git a/tool/ruby_vm/models/bare_instructions.rb b/tool/ruby_vm/models/bare_instructions.rb
index 6d595ebaf5..bee93f4334 100755
--- a/tool/ruby_vm/models/bare_instructions.rb
+++ b/tool/ruby_vm/models/bare_instructions.rb
@@ -140,7 +140,6 @@ def generate_attribute t, k, v
       type: t,                          \
       location: [],                     \
       expr: v.to_s + ';'
-    return @attrs[k] ||= attr
   end
 
   def predefine_attributes
diff --git a/tool/ruby_vm/models/instructions.rb b/tool/ruby_vm/models/instructions.rb
index 1198c7a4a6..36e0ec0caa 100644
--- a/tool/ruby_vm/models/instructions.rb
+++ b/tool/ruby_vm/models/instructions.rb
@@ -18,5 +18,6 @@
                        RubyVM::OperandsUnifications.to_a + \
                        RubyVM::InstructionsUnifications.to_a
 
+require_relative 'sendpop_instructions'
 require_relative 'trace_instructions'
 RubyVM::Instructions.freeze
diff --git a/tool/ruby_vm/models/sendpop_instructions.rb b/tool/ruby_vm/models/sendpop_instructions.rb
new file mode 100644
index 0000000000..5ad809b8e5
--- /dev/null
+++ b/tool/ruby_vm/models/sendpop_instructions.rb
@@ -0,0 +1,41 @@
+#! /your/favourite/path/to/ruby
+# -*- Ruby -*-
+# -*- frozen_string_literal: true; -*-
+# -*- warn_indent: true; -*-
+#
+# Copyright (c) 2019 Urabe, Shyouhei.  All rights reserved.
+#
+# This file is  a part of the programming language  Ruby.  Permission is hereby
+# granted, to  either redistribute and/or  modify this file, provided  that the
+# conditions  mentioned in  the file  COPYING are  met.  Consult  the file  for
+# details.
+
+require_relative 'bare_instructions'
+require_relative 'typemap'
+
+class RubyVM::SendpopInstructions < RubyVM::BareInstructions
+  def initialize orig
+    str = Marshal.dump orig.template
+    json = Marshal.load str
+    json[:name] = "opt_sendpop_#{orig.name}"
+    json[:signature][:ret] = []
+    super json
+    # The original return value should be declared -- though it is discarded --
+    # to prevent compile errors.
+    orig.rets.each {|v| @variables[v[:name]] = v }
+  end
+
+  private
+
+  re = %r/#{RubyVM::Typemap["CALL_INFO"].first}/o # => /C/
+  @instances =                            \
+    RubyVM::Instructions                  \
+    . select {|i| re =~ i.operands_info } \
+    . map    {|i| new i }
+
+  def self.to_a
+    @instances
+  end
+
+  RubyVM::Instructions.push(*to_a)
+end
diff --git a/tool/ruby_vm/views/_insn_entry.erb b/tool/ruby_vm/views/_insn_entry.erb
index b790729611..144a82e344 100644
--- a/tool/ruby_vm/views/_insn_entry.erb
+++ b/tool/ruby_vm/views/_insn_entry.erb
@@ -6,6 +6,7 @@
 %# conditions mentioned  in the  file COPYING  are met.   Consult the  file for
 %# details.
 %;
+%klass = insn.class.name.sub(/RubyVM::/, '').gsub(/([a-z])([A-Z])/, '\\1_\\2')
 
 /* insn <%= insn.pretty_name %> */
 INSN_ENTRY(<%= insn.name %>)
@@ -13,6 +14,7 @@ INSN_ENTRY(<%= insn.name %>)
 %#  NAME_OF_CURRENT_INSN is used in vm_exec.h
 #   define NAME_OF_CURRENT_INSN <%= insn.name %>
 #   define INSN_ATTR(x) <%= insn.call_attribute(' ## x ## ') %>
+#   define INSN_CLASS RUBYVM_<%= upcase klass %>
     bool leaf;
     MAYBE_UNUSED(VALUE *) canary;
 % unless insn.declarations.empty?
@@ -55,6 +57,7 @@ INSN_ENTRY(<%= insn.name %>)
 % end
     if (leaf) ADD_PC(INSN_ATTR(width));
     END_INSN(<%= insn.name %>);
+#   undef INSN_CLASS
 #   undef INSN_ATTR
 #   undef NAME_OF_CURRENT_INSN
 }
diff --git a/tool/ruby_vm/views/_sp_inc_helpers.erb b/tool/ruby_vm/views/_sp_inc_helpers.erb
index 550b9a4247..2d2dff9290 100644
--- a/tool/ruby_vm/views/_sp_inc_helpers.erb
+++ b/tool/ruby_vm/views/_sp_inc_helpers.erb
@@ -18,18 +18,21 @@ sp_inc_of_sendish(const struct rb_call_info *ci)
 {
     /* Send-ish instructions will:
      *
-     * 1. Pop block argument, if any.
-     * 2. Pop ordinal arguments.
-     * 3. Pop receiver.
-     * 4. Push return value.
+     * 1. If there is a block arg, pop it from the stack.
+     * 2. Pop other arguments from the stack.
+     * 3. Pop receiver from the stack.
+     * 4. (The actual evaluation goes here...)
+     * 5. Then, push the return value onto the stack.
+     * 6. Optionally, immediately pop that value if desired.
      */
     const int argb = (ci->flag & VM_CALL_ARGS_BLOCKARG) ? 1 : 0;
     const int argc = ci->orig_argc;
     const int recv = 1;
     const int retn = 1;
+    const int popn = (ci->flag & VM_CALL_POPPED) ? 1 : 0;
 
-    /*         1.     2.     3.     4. */
-    return 0 - argb - argc - recv + retn;
+    /*         1.     2.     3.     5.     6.    */
+    return 0 - argb - argc - recv + retn - popn;
 }
 
 #ifdef INSIDE_VM_INSNHELPER_C
diff --git a/tool/ruby_vm/views/vm.inc.erb b/tool/ruby_vm/views/vm.inc.erb
index c1a3faf60a..5fc2d53a2d 100644
--- a/tool/ruby_vm/views/vm.inc.erb
+++ b/tool/ruby_vm/views/vm.inc.erb
@@ -13,6 +13,15 @@
 } -%>
 
 #include "vm_insnhelper.h"
+% RubyVM                                           \
+% . constants                                      \
+% . map {|i| i.to_s }                              \
+% . grep(/.(Instructions|Unifications)$/)          \
+% . map {|i| i.gsub(/([a-z])([A-Z])/, '\\1_\\2') } \
+% . sort                                           \
+% . each_with_index do |i, j|
+#define RUBYVM_<%=upcase i %> <%= j %>
+% end
 % RubyVM::BareInstructions.to_a.each do |insn|
 <%= render 'insn_entry', locals: { insn: insn } -%>
 % end
@@ -25,6 +34,10 @@
 <%= render 'insn_entry', locals: { insn: insn } -%>
 % end
 %
+% RubyVM::SendpopInstructions.to_a.each do |insn|
+<%= render 'insn_entry', locals: { insn: insn } -%>
+% end
+%
 % RubyVM::TraceInstructions.to_a.each do |insn|
 <%= render 'trace_instruction', locals: { insn: insn } -%>
 % end
diff --git a/vm_core.h b/vm_core.h
index 513b8b85c1..6b16c44487 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -1077,6 +1077,7 @@ enum vm_call_flag_bits {
     VM_CALL_SUPER_bit,          /* super */
     VM_CALL_ZSUPER_bit,         /* zsuper */
     VM_CALL_OPT_SEND_bit,       /* internal flag */
+    VM_CALL_POPPED_bit,         /* internal flag */
     VM_CALL__END
 };
 
@@ -1092,6 +1093,7 @@ enum vm_call_flag_bits {
 #define VM_CALL_SUPER           (0x01 << VM_CALL_SUPER_bit)
 #define VM_CALL_ZSUPER          (0x01 << VM_CALL_ZSUPER_bit)
 #define VM_CALL_OPT_SEND        (0x01 << VM_CALL_OPT_SEND_bit)
+#define VM_CALL_POPPED          (0x01 << VM_CALL_POPPED_bit)
 
 enum vm_special_object_type {
     VM_SPECIAL_OBJECT_VMCORE = 1,
@@ -1133,11 +1135,11 @@ typedef rb_control_frame_t *
 
 enum {
     /* Frame/Environment flag bits:
-     *   MMMM MMMM MMMM MMMM ____ _FFF FFFF EEEX (LSB)
+     *   MMMM MMMM MMMM MMMM ____ FFFF FFFF EEEX (LSB)
      *
      * X   : tag for GC marking (It seems as Fixnum)
      * EEE : 3 bits Env flags
-     * FF..: 7 bits Frame flags
+     * FF..: 8 bits Frame flags
      * MM..: 15 bits frame magic (to check frame corruption)
      */
 
@@ -1162,6 +1164,7 @@ enum {
     VM_FRAME_FLAG_LAMBDA    = 0x0100,
     VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM = 0x0200,
     VM_FRAME_FLAG_POPPED    = 0x0400,
+    VM_FRAME_FLAG_POPIT     = 0x0800,
 
     /* env flag */
     VM_ENV_FLAG_LOCAL       = 0x0002,
diff --git a/vm_dump.c b/vm_dump.c
index 1d36eb0690..e5cca3ddff 100644
--- a/vm_dump.c
+++ b/vm_dump.c
@@ -131,6 +131,12 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c
     if (VM_FRAME_FINISHED_P(cfp)) {
 	fprintf(stderr, " [FINISH]");
     }
+    if (VM_ENV_FLAGS(cfp->ep, VM_FRAME_FLAG_POPPED)) {
+        fprintf(stderr, " [POPPED]");
+    }
+    if (VM_ENV_FLAGS(cfp->ep, VM_FRAME_FLAG_POPIT)) {
+        fprintf(stderr, " [POPIT]");
+    }
     if (0) {
 	fprintf(stderr, "              \t");
 	fprintf(stderr, "iseq: %-24s ", iseq_name);
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 3b3616810c..73f9c4a0d7 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -3302,12 +3302,12 @@ vm_sendish(
     VALUE val;
     int argc = ci->orig_argc;
     VALUE recv = TOPN(argc);
-    struct rb_calling_info calling;
-
-    calling.block_handler = block_handler;
-    calling.recv = recv;
-    calling.argc = argc;
-    calling.popped = popped;
+    struct rb_calling_info calling = {
+        .block_handler = block_handler,
+        .recv = recv,
+        .argc = argc,
+        .popped = popped,
+    };
 
     method_explorer(GET_CFP(), ci, cc, recv);
 
diff --git a/vm_insnhelper.h b/vm_insnhelper.h
index 4cf0c5da85..014bedc555 100644
--- a/vm_insnhelper.h
+++ b/vm_insnhelper.h
@@ -185,6 +185,19 @@ enum vm_regan_acttype {
 # define INC_GLOBAL_TIMESTAMP() (++ruby_vm_global_timestamp)
 #endif
 
+/* INSN_CLASS changes from place to place.  That prevents us from
+ * writing `#if INSN_CLASS == ...` right here. */
+#define INSN_CALLER_RETVAL_POPPED_P() \
+    (INSN_CLASS == RUBYVM_SENDPOP_INSTRUCTIONS) ? \
+        VM_FRAME_FLAG_POPPED : \
+    CURRENT_INSN_IS(pop) ? \
+        VM_FRAME_FLAG_POPPED : \
+    CURRENT_INSN_IS(leave) ? \
+        VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) : \
+    CURRENT_INSN_IS(opt_bailout) ? \
+        VM_ENV_FLAGS(GET_EP(), VM_FRAME_FLAG_POPPED) : \
+        0
+
 static inline struct vm_throw_data *
 THROW_DATA_NEW(VALUE val, const rb_control_frame_t *cf, VALUE st)
 {
-- 
2.17.1