* vm_core.h: remove rb_control_frame_t::bp (bp: base pointer).

  `bp' can be calculate by `sp' (stack pointer) of previous frame.
  Now, `bp_check' field is remained for debug.  You can eliminate
  this field by setting  VM_DEBUG_BP_CHECK as 0.
* vm_insnhelper.c (vm_base_ptr): add `vm_base_ptr(cfp).
  This function calculates base pointer from cfp.
* vm_insnhelper.c (vm_setup_method): push `recv' value on top of
  value stack (before method parameters).
  This change is for keeping consistency with normal method dispatch.
* insns.def: fix to use vm_base_ptr().
* vm.c (vm_exec): ditto.
* vm_dump.c: remove `bp' related dumps.
* cont.c (fiber_init): fix to check VM_DEBUG_BP_CHECK.



git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@37043 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

7 files changed, 98 insertions, 42 deletions

diff --git a/ChangeLog b/ChangeLog
index 082d46695b..7f26f53e8c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,25 @@
+Fri Sep 28 12:51:54 2012  Koichi Sasada  <ko1@atdot.net>
+
+	* vm_core.h: remove rb_control_frame_t::bp (bp: base pointer).
+	  `bp' can be calculate by `sp' (stack pointer) of previous frame.
+	  Now, `bp_check' field is remained for debug.  You can eliminate
+	  this field by setting  VM_DEBUG_BP_CHECK as 0.
+
+	* vm_insnhelper.c (vm_base_ptr): add `vm_base_ptr(cfp).
+	  This function calculates base pointer from cfp.
+
+	* vm_insnhelper.c (vm_setup_method): push `recv' value on top of
+	  value stack (before method parameters).
+	  This change is for keeping consistency with normal method dispatch.
+
+	* insns.def: fix to use vm_base_ptr().
+
+	* vm.c (vm_exec): ditto.
+
+	* vm_dump.c: remove `bp' related dumps.
+
+	* cont.c (fiber_init): fix to check VM_DEBUG_BP_CHECK.
+
 Fri Sep 28 10:40:51 2012  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* io.c (rb_io_reopen): accept File::Constants as well as mode string.
diff --git a/cont.c b/cont.c
index 548b163f7b..9cac2682df 100644
--- a/cont.c
+++ b/cont.c
@@ -1061,7 +1061,9 @@ fiber_init(VALUE fibval, VALUE proc)
     th->cfp--;
     th->cfp->pc = 0;
     th->cfp->sp = th->stack + 1;
-    th->cfp->bp = 0;
+#if VM_DEBUG_BP_CHECK
+    th->cfp->bp_check = 0;
+#endif
     th->cfp->ep = th->stack;
     *th->cfp->ep = VM_ENVVAL_BLOCK_PTR(0);
     th->cfp->self = Qnil;
diff --git a/insns.def b/insns.def
index 1dc4328d72..bbbac396be 100644
--- a/insns.def
+++ b/insns.def
@@ -1092,9 +1092,9 @@ leave
 (VALUE val)
 {
     if (OPT_CHECKED_RUN) {
-	if (reg_cfp->sp != reg_cfp->bp) {
+	if (reg_cfp->sp != vm_base_ptr(reg_cfp)) {
 	    rb_bug("Stack consistency error (sp: %"PRIdPTRDIFF", bp: %"PRIdPTRDIFF")",
-		   VM_SP_CNT(th, reg_cfp->sp), VM_SP_CNT(th, reg_cfp->bp));
+		   VM_SP_CNT(th, reg_cfp->sp), VM_SP_CNT(th, vm_base_ptr(reg_cfp)));
 	}
     }
 
@@ -1316,8 +1316,8 @@ opt_checkenv
 ()
 ()
 {
-    if (GET_CFP()->bp != GET_EP() + 1) {
-	VALUE *ep = GET_CFP()->bp - 1;
+    if (vm_base_ptr(reg_cfp) != GET_EP() + 1) {
+	VALUE *ep = vm_base_ptr(reg_cfp) - 1;
 	/* TODO: copy env and clean stack at creating env? */
 	*ep = *GET_EP();
 	SET_EP(ep);
diff --git a/vm.c b/vm.c
index c59ef0b895..348d1349fa 100644
--- a/vm.c
+++ b/vm.c
@@ -1272,7 +1272,7 @@ vm_exec(rb_thread_t *th)
 		    }
 		    else if (entry->type == type) {
 			cfp->pc = cfp->iseq->iseq_encoded + entry->cont;
-			cfp->sp = cfp->bp + entry->sp;
+			cfp->sp = vm_base_ptr(cfp) + entry->sp;
 
 			if (state != TAG_REDO) {
 #if OPT_STACK_CACHING
@@ -1317,7 +1317,7 @@ vm_exec(rb_thread_t *th)
 
 	    /* enter catch scope */
 	    GetISeqPtr(catch_iseqval, catch_iseq);
-	    cfp->sp = cfp->bp + cont_sp;
+	    cfp->sp = vm_base_ptr(cfp) + cont_sp;
 	    cfp->pc = cfp->iseq->iseq_encoded + cont_pc;
 
 	    /* push block frame */
diff --git a/vm_core.h b/vm_core.h
index d890c1619b..51af4770e1 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -353,18 +353,25 @@ typedef struct rb_vm_struct {
     VALUE *defined_strings;
 } rb_vm_t;
 
+#ifndef VM_DEBUG_BP_CHECK
+#define VM_DEBUG_BP_CHECK 0
+#endif
+
 typedef struct {
     VALUE *pc;			/* cfp[0] */
     VALUE *sp;			/* cfp[1] */
-    VALUE *bp;			/* cfp[2] */
-    rb_iseq_t *iseq;		/* cfp[3] */
-    VALUE flag;			/* cfp[4] */
-    VALUE self;			/* cfp[5] / block[0] */
-    VALUE klass;		/* cfp[6] / block[1] */
-    VALUE *ep;			/* cfp[7] / block[2] */
-    rb_iseq_t *block_iseq;	/* cfp[8] / block[3] */
-    VALUE proc;			/* cfp[9] / block[4] */
-    const rb_method_entry_t *me;/* cfp[10] */
+    rb_iseq_t *iseq;		/* cfp[2] */
+    VALUE flag;			/* cfp[3] */
+    VALUE self;			/* cfp[4] / block[0] */
+    VALUE klass;		/* cfp[5] / block[1] */
+    VALUE *ep;			/* cfp[6] / block[2] */
+    rb_iseq_t *block_iseq;	/* cfp[7] / block[3] */
+    VALUE proc;			/* cfp[8] / block[4] */
+    const rb_method_entry_t *me;/* cfp[9] */
+
+#if VM_DEBUG_BP_CHECK
+    VALUE *bp_check;		/* cfp[10] */
+#endif
 } rb_control_frame_t;
 
 typedef struct rb_block_struct {
@@ -707,7 +714,8 @@ rb_block_t *rb_vm_control_frame_block_ptr(rb_control_frame_t *cfp);
 
 #define RUBY_VM_GET_BLOCK_PTR_IN_CFP(cfp) ((rb_block_t *)(&(cfp)->self))
 #define RUBY_VM_GET_CFP_FROM_BLOCK_PTR(b) \
-  ((rb_control_frame_t *)((VALUE *)(b) - 5))
+  ((rb_control_frame_t *)((VALUE *)(b) - 4))
+/* magic number `4' is depend on rb_control_frame_t layout. */
 
 /* VM related object allocate functions */
 VALUE rb_thread_alloc(VALUE klass);
diff --git a/vm_dump.c b/vm_dump.c
index 99da986d7e..4f7525e8a9 100644
--- a/vm_dump.c
+++ b/vm_dump.c
@@ -27,7 +27,7 @@
 static void
 control_frame_dump(rb_thread_t *th, rb_control_frame_t *cfp)
 {
-    ptrdiff_t pc = -1, bp = -1;
+    ptrdiff_t pc = -1;
     ptrdiff_t ep = cfp->ep - th->stack;
     char ep_in_heap = ' ';
     char posbuf[MAX_POSBUF+1];
@@ -44,9 +44,6 @@ control_frame_dump(rb_thread_t *th, rb_control_frame_t *cfp)
 	ep = (ptrdiff_t)cfp->ep;
 	ep_in_heap = 'p';
     }
-    if (cfp->bp) {
-	bp = cfp->bp - th->stack;
-    }
 
     switch (VM_FRAME_TYPE(cfp)) {
       case VM_FRAME_MAGIC_TOP:
@@ -119,7 +116,7 @@ control_frame_dump(rb_thread_t *th, rb_control_frame_t *cfp)
     else {
 	fprintf(stderr, "p:%04"PRIdPTRDIFF" ", pc);
     }
-    fprintf(stderr, "s:%04"PRIdPTRDIFF" b:%04"PRIdPTRDIFF" ", (cfp->sp - th->stack), bp);
+    fprintf(stderr, "s:%04"PRIdPTRDIFF" ", cfp->sp - th->stack);
     fprintf(stderr, ep_in_heap == ' ' ? "e:%06"PRIdPTRDIFF" " : "e:%06"PRIxPTRDIFF" ", ep % 10000);
     fprintf(stderr, "%-6s", magic);
     if (line) {
@@ -141,7 +138,7 @@ void
 rb_vmdebug_stack_dump_raw(rb_thread_t *th, rb_control_frame_t *cfp)
 {
 #if 0
-    VALUE *sp = cfp->sp, *bp = cfp->bp, *ep = cfp->ep;
+    VALUE *sp = cfp->sp, *ep = cfp->ep;
     VALUE *p, *st, *t;
 
     fprintf(stderr, "-- stack frame ------------\n");
@@ -155,8 +152,6 @@ rb_vmdebug_stack_dump_raw(rb_thread_t *th, rb_control_frame_t *cfp)
 
 	if (p == ep)
 	    fprintf(stderr, " <- ep");
-	if (p == bp)
-	    fprintf(stderr, " <- bp");	/* should not be */
 
 	fprintf(stderr, "\n");
     }
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 46ef7e1a1c..808d73cb9f 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -57,7 +57,9 @@ vm_push_frame(rb_thread_t *th,
 
     cfp->pc = (VALUE *)pc;
     cfp->sp = sp + 1;
-    cfp->bp = sp + 1;
+#if VM_DEBUG_BP_CHECK
+    cfp->bp_check = sp + 1;
+#endif
     cfp->ep = sp;
     cfp->iseq = (rb_iseq_t *) iseq;
     cfp->flag = type;
@@ -494,19 +496,16 @@ vm_setup_method(rb_thread_t *th, rb_control_frame_t *cfp,
 		const rb_method_entry_t *me, VALUE defined_class)
 {
     int opt_pc, i;
-    VALUE *sp, *rsp = cfp->sp - argc;
+    VALUE *sp, *argv = cfp->sp - argc;
     rb_iseq_t *iseq = me->def->body.iseq;
 
-    VM_CALLEE_SETUP_ARG(opt_pc, th, iseq, argc, rsp, &blockptr);
+    VM_CALLEE_SETUP_ARG(opt_pc, th, iseq, argc, argv, &blockptr);
 
     /* stack overflow check */
     CHECK_STACK_OVERFLOW(cfp, iseq->stack_max);
-
-    sp = rsp + iseq->arg_size;
+    sp = argv + iseq->arg_size;
 
     if (LIKELY(!(flag & VM_CALL_TAILCALL_BIT))) {
-	if (0) printf("local_size: %d, arg_size: %d\n",
-		      iseq->local_size, iseq->arg_size);
 
 	/* clear local variables */
 	for (i = 0; i < iseq->local_size - iseq->arg_size; i++) {
@@ -517,30 +516,36 @@ vm_setup_method(rb_thread_t *th, rb_control_frame_t *cfp,
 		      VM_ENVVAL_BLOCK_PTR(blockptr),
 		      iseq->iseq_encoded + opt_pc, sp, 0, me);
 
-	cfp->sp = rsp - 1 /* recv */;
+	cfp->sp = argv - 1 /* recv */;
     }
     else {
-	VALUE *p_rsp;
-	int is_finish_frame = VM_FRAME_TYPE_FINISH_P(cfp);
+	VALUE *src_argv = argv;
+	VALUE *sp_orig;
+	int src_argc = sp - src_argv;
+	VALUE finish_flag = VM_FRAME_TYPE_FINISH_P(cfp) ? VM_FRAME_FLAG_FINISH : 0;
+	cfp = th->cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(th->cfp); /* pop cf */
 
-	th->cfp++; /* pop cf */
-	p_rsp = th->cfp->sp;
+	sp = sp_orig = cfp->sp;
+
+	/* push self */
+	sp[0] = recv;
+	sp++;
 
 	/* copy arguments */
-	for (i=0; i < (sp - rsp); i++) {
-	    p_rsp[i] = rsp[i];
+	for (i=0; i < src_argc; i++) {
+	    *sp++ = src_argv[i];
 	}
 
-	sp -= rsp - p_rsp;
-
 	/* clear local variables */
 	for (i = 0; i < iseq->local_size - iseq->arg_size; i++) {
 	    *sp++ = Qnil;
 	}
 
-	vm_push_frame(th, iseq, VM_FRAME_MAGIC_METHOD | (is_finish_frame ? VM_FRAME_FLAG_FINISH : 0),
+	vm_push_frame(th, iseq, VM_FRAME_MAGIC_METHOD | finish_flag,
 		      recv, defined_class, VM_ENVVAL_BLOCK_PTR(blockptr),
 		      iseq->iseq_encoded + opt_pc, sp, 0, me);
+
+	cfp->sp = sp_orig;
     }
 }
 
@@ -1902,3 +1907,27 @@ double_cmp_ge(double a, double b)
     CHECK_CMP_NAN(a, b);
     return a >= b ? Qtrue : Qfalse;
 }
+
+static VALUE *
+vm_base_ptr(rb_control_frame_t *cfp)
+{
+    rb_control_frame_t *prev_cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp);
+    VALUE *bp = prev_cfp->sp + cfp->iseq->local_size + 1;
+
+    if (cfp->iseq->type == ISEQ_TYPE_METHOD) {
+	/* adjust `self' */
+	bp += 1;
+    }
+
+#if VM_DEBUG_BP_CHECK
+    if (bp != cfp->bp_check) {
+	fprintf(stderr, "bp_check: %d, bp: %d\n",
+		cfp->bp_check - GET_THREAD()->stack,
+		bp - GET_THREAD()->stack);
+	rb_bug("vm_base_ptr: unreachable");
+    }
+#endif
+
+    return bp;
+}
+