Bug #16161: tailcall_optimization may be disabled after r67315

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: -
• ruby -v: ruby 2.7.0dev (2019-09-09T23:18:03Z master 3678c37119) [x86_64-linux]
• Backport: 2.5: DONTNEED, 2.6: DONTNEED

[wanabe (_ wanabe)]

Before r67315:

$ ./miniruby -v -e 'iseq = RubyVM::InstructionSequence.compile("def foo(n, s = 0);return s if n < 1;foo(n - 1, n + s); end", tailcall_optimization: true); iseq.eval; p foo(900_000)'
ruby 2.7.0dev (2019-03-20 trunk 67314) [x86_64-linux]
405000450000

After r67315:

$ ./miniruby -v -e 'iseq = RubyVM::InstructionSequence.compile("def foo(n, s = 0);return s if n < 1;foo(n - 1, n + s); end", tailcall_optimization: true); iseq.eval; p foo(900_000)'
ruby 2.7.0dev (2019-03-20 trunk 67315) [x86_64-linux]
Traceback (most recent call last):
	10080: from -e:1:in `<main>'
	10079: from <compiled>:1:in `foo'
	10078: from <compiled>:1:in `foo'
	10077: from <compiled>:1:in `foo'
	10076: from <compiled>:1:in `foo'
	10075: from <compiled>:1:in `foo'
	10074: from <compiled>:1:in `foo'
	10073: from <compiled>:1:in `foo'
	 ... 10068 levels...
	    4: from <compiled>:1:in `foo'
	    3: from <compiled>:1:in `foo'
	    2: from <compiled>:1:in `foo'
	    1: from <compiled>:1:in `foo'
<compiled>:1:in `foo': stack level too deep (SystemStackError)

master:

$ ./miniruby -v -e 'iseq = RubyVM::InstructionSequence.compile("def foo(n, s = 0);return s if n < 1;foo(n - 1, n + s); end", tailcall_optimization: true); iseq.eval; p foo(900_000)'
ruby 2.7.0dev (2019-09-09T23:18:03Z master 3678c37119) [x86_64-linux]
Traceback (most recent call last):
	10080: from -e:1:in `<main>'
	10079: from <compiled>:1:in `foo'
	10078: from <compiled>:1:in `foo'
	10077: from <compiled>:1:in `foo'
	10076: from <compiled>:1:in `foo'
	10075: from <compiled>:1:in `foo'
	10074: from <compiled>:1:in `foo'
	10073: from <compiled>:1:in `foo'
	 ... 10068 levels...
	    4: from <compiled>:1:in `foo'
	    3: from <compiled>:1:in `foo'
	    2: from <compiled>:1:in `foo'
	    1: from <compiled>:1:in `foo'
<compiled>:1:in `foo': stack level too deep (SystemStackError)

I think ruby should not raise SystemStackError with tailcall_optimization, should it?

[wanabe (_ wanabe)]

How about adding vm_call_iseq_setup_tailcall_opt_start as same as vm_call_iseq_setup_normal_opt_start?
(Copy-and-paste code, sorry!)

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index a83e3a952a..4abee2fadd 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1825,6 +1825,30 @@ vm_call_iseq_setup_normal_opt_start(rb_execution_context_t *ec, rb_control_frame
     return vm_call_iseq_setup_normal(ec, cfp, calling, cc->me, opt_pc, param - delta, local);
 }
 
+static VALUE
+vm_call_iseq_setup_tailcall_opt_start(rb_execution_context_t *ec, rb_control_frame_t *cfp,
+                                      struct rb_calling_info *calling,
+                                      const struct rb_call_info *ci, struct rb_call_cache *cc)
+{
+    const rb_iseq_t *iseq = def_iseq_ptr(cc->me->def);
+    const int lead_num = iseq->body->param.lead_num;
+    const int opt = calling->argc - lead_num;
+    const int opt_pc = (int)iseq->body->param.opt_table[opt];
+
+    RB_DEBUG_COUNTER_INC(ccf_iseq_opt);
+
+#if USE_OPT_HIST
+    if (opt_pc < OPT_HIST_MAX) {
+        opt_hist[opt]++;
+    }
+    else {
+        opt_hist[OPT_HIST_MAX]++;
+    }
+#endif
+
+    return vm_call_iseq_setup_tailcall(ec, cfp, calling, ci, cc, opt_pc);
+}
+
 static void
 args_setup_kw_parameters(rb_execution_context_t *const ec, const rb_iseq_t *const iseq,
                          VALUE *const passed_values, const int passed_keyword_len, const VALUE *const passed_keywords,
@@ -1909,9 +1933,16 @@ vm_callee_setup_arg(rb_execution_context_t *ec, struct rb_calling_info *calling,
                 argument_arity_error(ec, iseq, argc, lead_num, lead_num + opt_num);
             }
 
-            CC_SET_FASTPATH(cc, vm_call_iseq_setup_normal_opt_start,
-                            !IS_ARGS_SPLAT(ci) && !IS_ARGS_KEYWORD(ci) &&
-                            !(METHOD_ENTRY_VISI(cc->me) == METHOD_VISI_PROTECTED));
+            if (LIKELY(!(ci->flag & VM_CALL_TAILCALL))) {
+                CC_SET_FASTPATH(cc, vm_call_iseq_setup_normal_opt_start,
+                                !IS_ARGS_SPLAT(ci) && !IS_ARGS_KEYWORD(ci) &&
+                                !(METHOD_ENTRY_VISI(cc->me) == METHOD_VISI_PROTECTED));
+            }
+            else {
+                CC_SET_FASTPATH(cc, vm_call_iseq_setup_tailcall_opt_start,
+                                !IS_ARGS_SPLAT(ci) && !IS_ARGS_KEYWORD(ci) &&
+                                !(METHOD_ENTRY_VISI(cc->me) == METHOD_VISI_PROTECTED));
+            }
 
             /* initialize opt vars for self-references */
             VM_ASSERT((int)iseq->body->param.size == lead_num + opt_num);

[wanabe (_ wanabe)]

• Assignee set to ko1 (Koichi Sasada)

[wanabe (_ wanabe)]

I created Pull-Request https://github.com/ruby/ruby/pull/2528

[wanabe (_ wanabe)]

Oh I've pushed wrong commit on https://github.com/ruby/ruby/pull/2528 .
I retry to push my working branch on https://github.com/ruby/ruby/pull/2529 .
Sorry!

[wanabe (_ wanabe)]

• Status changed from Open to Closed

Thank you to merge https://github.com/ruby/ruby/pull/2529 on 4ff2c58f919153b9a47f69f855a0b9d2bb0e0bbe.
I confirmed this issue is fixed now.

$ ./miniruby -v -e 'iseq = RubyVM::InstructionSequence.compile("def foo(n, s = 0);return s if n < 1;foo(n - 1, n + s); end", tailcall_optimization: true); iseq.eval; p foo(900_000)'
ruby 2.7.0dev (2019-10-25T01:45:46Z master a7ec88ad61) [x86_64-linux]
405000450000

[wanabe (_ wanabe)]

• Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN to 2.5: DONTNEED, 2.6: DONTNEED