k0kubun	2018-05-03 12:08:21 +0900 (Thu, 03 May 2018)

  New Revision: 63331

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63331

  Log:
    mjit_compile.c: verify stack size agreement
    
    between branches.
    
    mjit_compile.inc.erb: move the compiled_for_pos reference to
    mjit_compile.c

  Modified files:
    trunk/mjit_compile.c
    trunk/tool/ruby_vm/views/mjit_compile.inc.erb
Index: tool/ruby_vm/views/mjit_compile.inc.erb
===================================================================
--- tool/ruby_vm/views/mjit_compile.inc.erb	(revision 63330)
+++ tool/ruby_vm/views/mjit_compile.inc.erb	(revision 63331)
@@ -73,7 +73,3 @@ switch (insn) { https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/mjit_compile.inc.erb#L73
     status->success = FALSE;
     break;
 }
-
-/* if next_pos is already compiled, next instruction won't be compiled in C code and needs `goto`. */
-if ((next_pos < body->iseq_size && status->compiled_for_pos[next_pos]))
-    fprintf(f, "goto label_%d;\n", next_pos);
Index: mjit_compile.c
===================================================================
--- mjit_compile.c	(revision 63330)
+++ mjit_compile.c	(revision 63331)
@@ -14,12 +14,16 @@ https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L14
 #include "insns_info.inc"
 #include "vm_insnhelper.h"
 
+/* Macros to check if a position is already compiled using compile_status.stack_size_for_pos */
+#define NOT_COMPILED_STACK_SIZE -1
+#define ALREADY_COMPILED_P(status, pos) (status->stack_size_for_pos[pos] != NOT_COMPILED_STACK_SIZE)
+
 /* Storage to keep compiler's status.  This should have information
    which is global during one `mjit_compile` call.  Ones conditional
    in each branch should be stored in `compile_branch`.  */
 struct compile_status {
     int success; /* has TRUE if compilation has had no issue */
-    int *compiled_for_pos; /* compiled_for_pos[pos] has TRUE if the pos is compiled */
+    int *stack_size_for_pos; /* stack_size_for_pos[pos] has stack size for the position (otherwise -1) */
     /* If TRUE, JIT-ed code will use local variables to store pushed values instead of
        using VM's stack and moving stack pointer. */
     int local_stack_p;
@@ -118,6 +122,20 @@ compile_insn(FILE *f, const struct rb_is https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L122
  #include "mjit_compile.inc"
 /*****************/
 
+    if (next_pos < body->iseq_size && ALREADY_COMPILED_P(status, next_pos)) {
+        /* Verify stack size assumption is the same among multiple branches */
+        if ((unsigned int)status->stack_size_for_pos[next_pos] != b->stack_size) {
+            if (mjit_opts.warnings || mjit_opts.verbose)
+                fprintf(stderr, "MJIT warning: JIT stack assumption is not the same between branches (%d != %u)\n",
+                        status->stack_size_for_pos[next_pos], b->stack_size);
+            status->success = FALSE;
+            return next_pos;
+        }
+
+        /* If next_pos is already compiled, next instruction won't be compiled in C code and needs `goto`. */
+        fprintf(f, "goto label_%d;\n", next_pos);
+    }
+
     return next_pos;
 }
 
@@ -133,13 +151,13 @@ compile_insns(FILE *f, const struct rb_i https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L151
     branch.stack_size = stack_size;
     branch.finish_p = FALSE;
 
-    while (pos < body->iseq_size && !status->compiled_for_pos[pos] && !branch.finish_p) {
+    while (pos < body->iseq_size && !ALREADY_COMPILED_P(status, pos) && !branch.finish_p) {
 #if OPT_DIRECT_THREADED_CODE || OPT_CALL_THREADED_CODE
         insn = rb_vm_insn_addr2insn((void *)body->iseq_encoded[pos]);
 #else
         insn = (int)body->iseq_encoded[pos];
 #endif
-        status->compiled_for_pos[pos] = TRUE;
+        status->stack_size_for_pos[pos] = (int)branch.stack_size;
 
         fprintf(f, "\nlabel_%d: /* %s */\n", pos, insn_name(insn));
         pos = compile_insn(f, body, insn, body->iseq_encoded + (pos+1), pos, status, &branch);
@@ -173,8 +191,9 @@ mjit_compile(FILE *f, const struct rb_is https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L191
 {
     struct compile_status status;
     status.success = TRUE;
-    status.compiled_for_pos = ZALLOC_N(int, body->iseq_size);
     status.local_stack_p = !body->catch_except_p;
+    status.stack_size_for_pos = ALLOC_N(int, body->iseq_size);
+    memset(status.stack_size_for_pos, NOT_COMPILED_STACK_SIZE, sizeof(int) * body->iseq_size);
 
     /* For performance, we verify stack size only on compilation time (mjit_compile.inc.erb) without --jit-debug */
     if (!mjit_opts.debug) {
@@ -217,6 +236,6 @@ mjit_compile(FILE *f, const struct rb_is https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L236
     compile_cancel_handler(f, body, &status);
     fprintf(f, "\n} /* end of %s */\n", funcname);
 
-    xfree(status.compiled_for_pos);
+    xfree(status.stack_size_for_pos);
     return status.success;
 }

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/