From e39eb9dab50eaa681467e51145b37cdc11667830 Mon Sep 17 00:00:00 2001 From: ko1 Date: Thu, 23 Aug 2007 07:10:56 +0000 Subject: [PATCH] * compile.c, insns.def, parse.y: fix massign order. This change causes performance problem. Try vm1_swap benchmark. [ruby-dev:31522] * insns.def, insnhelper.ci: move process body of expandarray insn to vm_expandarray(). * bootstraptest/test_knownbug.rb, bootstraptest/test_massign.rb: move a solved test. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@13236 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 12 +++ bootstraptest/test_knownbug.rb | 1 - bootstraptest/test_massign.rb | 1 + compile.c | 161 +++++++++------------------------ insnhelper.ci | 58 ++++++++++++ insns.def | 50 +--------- parse.y | 2 +- 7 files changed, 122 insertions(+), 163 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1c15034c8b5f03..4cf2718a7f90d6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +Thu Aug 23 16:04:11 2007 Koichi Sasada + + * compile.c, insns.def, parse.y: fix massign order. This change + causes performance problem. Try vm1_swap benchmark. + [ruby-dev:31522] + + * insns.def, insnhelper.ci: move process body of expandarray insn to + vm_expandarray(). + + * bootstraptest/test_knownbug.rb, bootstraptest/test_massign.rb: + move a solved test. + Thu Aug 23 15:51:19 2007 Nobuyoshi Nakada * parse.y (f_norm_arg): ripper has no shadowing check. diff --git a/bootstraptest/test_knownbug.rb b/bootstraptest/test_knownbug.rb index 6aa4c714e289a1..114916710c79e7 100644 --- a/bootstraptest/test_knownbug.rb +++ b/bootstraptest/test_knownbug.rb @@ -4,7 +4,6 @@ # # massign -assert_equal '2', 'a, a = 1, 2; a', "[ruby-dev:31522]" assert_equal '[0,1,{2=>3}]', '[0,*[1],2=>3]', "[ruby-dev:31592]" diff --git a/bootstraptest/test_massign.rb b/bootstraptest/test_massign.rb index 71b53452b9f78c..202dbd38e5034b 100644 --- a/bootstraptest/test_massign.rb +++ b/bootstraptest/test_massign.rb @@ -3,6 +3,7 @@ assert_equal '[]', '*a = *nil; a' assert_equal '[nil]', '*a = nil; a' +assert_equal '2', 'a, a = 1, 2; a', "[ruby-dev:31522]" =begin # generated by this script: diff --git a/compile.c b/compile.c index af8f85c81ea105..0a725cb68bee5f 100644 --- a/compile.c +++ b/compile.c @@ -1946,7 +1946,7 @@ when_vals(rb_iseq_t *iseq, LINK_ANCHOR *cond_seq, NODE *vals, LABEL *l1, VALUE s } static int -make_masgn_lhs(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE *node) +compile_massign_lhs(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE *node) { switch (nd_type(node)) { case NODE_ATTRASGN: { @@ -1968,7 +1968,11 @@ make_masgn_lhs(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE *node) break; } case NODE_MASGN: { - COMPILE_POPED(ret, "nest masgn lhs", node); + DECL_ANCHOR(anchor); + INIT_ANCHOR(anchor); + COMPILE_POPED(anchor, "nest masgn lhs", node); + REMOVE_ELEM(FIRST_ELEMENT(anchor)); + ADD_SEQ(ret, anchor); break; } default: { @@ -1987,123 +1991,60 @@ make_masgn_lhs(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE *node) } static int -compile_massign(rb_iseq_t *iseq, LINK_ANCHOR *ret, - NODE *rhsn, NODE *splatn, NODE *lhsn, int llen) +compile_massign(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE *node, int poped) { - if (lhsn != 0) { - compile_massign(iseq, ret, rhsn, splatn, lhsn->nd_next, llen + 1); - make_masgn_lhs(iseq, ret, lhsn->nd_head); + NODE *rhsn = node->nd_value; + NODE *splatn = node->nd_args; + NODE *lhsn = node->nd_head; + int lhs_splat = (splatn && (VALUE)splatn != (VALUE)-1) ? 1 : 0; + + if (!poped && 0) { + /* optimized one */ + //compile_massign_opt(iseq, ret, rhsn, splatn, lhsn, llen); } else { - int lhs_splat = 0; - - if (splatn && (VALUE)splatn != -1) { - lhs_splat = 1; - } - - if (rhsn) { - switch (nd_type(rhsn)) { - case NODE_ARRAY:{ - int rlen = rhsn->nd_alen; - int max = rlen > llen ? rlen : llen; - int i, si = 0; - int rline = nd_line(rhsn); - - for (i = 0; i < max; i++) { - if (i < rlen && i < llen) { - /* a, b = c, d */ - COMPILE(ret, "masgn val1", rhsn->nd_head); - rline = nd_line(rhsn); - rhsn = rhsn->nd_next; - } - else if (i < rlen) { - if (lhs_splat) { - while (rhsn) { - /* a, *b = x, y, z */ - si++; - COMPILE(ret, "masgn rhs for lhs splat", - rhsn->nd_head); - rline = nd_line(rhsn); - rhsn = rhsn->nd_next; - } - break; - } - else { - /* a, b = c, d, e */ - COMPILE_POPED(ret, "masgn rhs (popped)", - rhsn->nd_head); - rhsn = rhsn->nd_next; - } - } - else if (i < llen) { - /* a, b, c = c, d */ - ADD_INSN(ret, rline, putnil); - } - } + DECL_ANCHOR(lhsseq); + int llen = 0; + INIT_ANCHOR(lhsseq); - if (lhs_splat) { - ADD_INSN1(ret, rline, newarray, INT2FIX(si)); - } - break; - } - case NODE_TO_ARY: - COMPILE(ret, "rhs to ary", rhsn->nd_head); - ADD_INSN2(ret, nd_line(rhsn), expandarray, INT2FIX(llen), - INT2FIX(lhs_splat)); - break; - - case NODE_SPLAT: - COMPILE(ret, "rhs to ary (splat)", rhsn); - ADD_INSN2(ret, nd_line(rhsn), expandarray, INT2FIX(llen), INT2FIX(lhs_splat)); - break; + while (lhsn) { + compile_massign_lhs(iseq, lhsseq, lhsn->nd_head); + llen += 1; + lhsn = lhsn->nd_next; + } - case NODE_ARGSCAT: - COMPILE(ret, "rhs to argscat", rhsn); - ADD_INSN2(ret, nd_line(rhsn), expandarray, INT2FIX(llen), INT2FIX(lhs_splat)); - break; + COMPILE(ret, "normal masgn rhs", rhsn); - default: - COMPILE(ret, "rhs to ary (splat/default)", rhsn); - ADD_INSN2(ret, nd_line(rhsn), expandarray, INT2FIX(llen), INT2FIX(lhs_splat)); - } - } - else { - /* nested massign */ - ADD_INSN2(ret, 0, expandarray, INT2FIX(llen), INT2FIX(lhs_splat)); + if (!poped) { + ADD_INSN(ret, nd_line(node), dup); } + ADD_INSN2(ret, nd_line(node), expandarray, + INT2FIX(llen), INT2FIX(lhs_splat)); + ADD_SEQ(ret, lhsseq); + if (lhs_splat) { if (nd_type(splatn) == NODE_POSTARG) { - NODE *n = splatn->nd_2nd; - int num = n->nd_alen; - - ADD_INSN (ret, nd_line(n), dup); - ADD_INSN2(ret, nd_line(n), expandarray, INT2FIX(num), INT2FIX(2)); + /*a, b, *r, p1, p2 */ + NODE *postn = splatn->nd_2nd; + NODE *restn = splatn->nd_1st; + int num = postn->nd_alen; + int flag = 0x02 | (((VALUE)restn == (VALUE)-1) ? 0x00 : 0x01); - while (n) { - DECL_ANCHOR(lhs); + ADD_INSN2(ret, nd_line(splatn), expandarray, + INT2FIX(num), INT2FIX(flag)); - INIT_ANCHOR(lhs); - COMPILE_POPED(lhs, "post", n->nd_head); - - if (nd_type(n->nd_head) != NODE_MASGN) { - REMOVE_ELEM(FIRST_ELEMENT(lhs)); - } - - ADD_SEQ(ret, lhs); - n = n->nd_next; + if ((VALUE)restn != (VALUE)-1) { + compile_massign_lhs(iseq, ret, restn); } - - if (splatn->nd_1st == (NODE*)(VALUE)-1) { - /* v1, *, v2 = expr */ - ADD_INSN(ret, nd_line(splatn), pop); - } - else { - make_masgn_lhs(iseq, ret, splatn->nd_1st); + while (postn) { + compile_massign_lhs(iseq, ret, postn->nd_head); + postn = postn->nd_next; } } else { - make_masgn_lhs(iseq, ret, splatn); + /* a, b, *r */ + compile_massign_lhs(iseq, ret, splatn); } } } @@ -3204,19 +3145,7 @@ iseq_compile_each(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE * node, int poped) } case NODE_MASGN:{ - if (poped) { - compile_massign(iseq, ret, - node->nd_value, /* rhsn */ - node->nd_args, /* splat */ - node->nd_head, /* lhsn */ - 0); - } - else { - COMPILE(ret, "masgn/value", node->nd_value); - ADD_INSN(ret, nd_line(node), dup); - compile_massign(iseq, ret, 0, - node->nd_args, node->nd_head, 0); - } + compile_massign(iseq, ret, node, poped); break; } diff --git a/insnhelper.ci b/insnhelper.ci index 530ccf97ab3623..4c52e5b01d5fbb 100644 --- a/insnhelper.ci +++ b/insnhelper.ci @@ -1321,6 +1321,64 @@ vm_throw(rb_thread_t *th, rb_control_frame_t *reg_cfp, rb_num_t throw_state, VAL } } +static inline void +vm_expandarray(rb_control_frame_t *cfp, VALUE ary, int num, int flag) +{ + int is_splat = flag & 0x01; + int space_size = num + is_splat; + VALUE *base = cfp->sp, *ptr; + int len; + + cfp->sp += space_size; + + if (TYPE(ary) != T_ARRAY) { + ary = rb_ary_to_ary(ary); + } + ptr = RARRAY_PTR(ary); + len = RARRAY_LEN(ary); + + if (flag & 0x02) { + /* post: ..., nil ,ary[-1], ..., ary[0..-num] # top */ + int i = 0, j; + + if (len < num) { + for (i=0; i len) { + *bptr = rb_ary_new(); + } + else { + *bptr = rb_ary_new4(len - num, ptr + num); + } + } + } +} + static void call_end_proc(VALUE data) { diff --git a/insns.def b/insns.def index 5728de6334517d..60d9d9a634a98d 100644 --- a/insns.def +++ b/insns.def @@ -446,57 +446,17 @@ duparray num以上の要素は切り捨てる。 配列オブジェクトでなければ、num - 1 個の nil を積む。 もし flag が真なら、残り要素の配列を積む + flag: 0x01 - 最後を配列に + flag: 0x02 - postarg 用 + flag: 0x04 - reverse? */ DEFINE_INSN expandarray (rb_num_t num, rb_num_t flag) (..., VALUE ary) -(...) // inc += flag == 2 ? num : ((num > 0) ? num - 1 + (flag ? 1 : 0) : num + 1 - (flag ? 1 : 0)); +(...) // inc += flag == 0x02 ? num : ((num > 0) ? num - 1 + (flag ? 1 : 0) : num + 1 - (flag ? 1 : 0)); { - int i; - - if (flag == 2) { - VALUE *ptr = RARRAY_PTR(ary); - int len = RARRAY_LEN(ary); - int start = len - num; - - if (start < 0) { - for (i=0; i num) { - PUSH(rb_ary_new4(len - num, &RARRAY_PTR(ary)[num])); - } - else { - PUSH(rb_ary_new()); - } - } - } + vm_expandarray(GET_CFP(), ary, num, flag); } /** diff --git a/parse.y b/parse.y index 64532067cd80eb..e76c7c3187274c 100644 --- a/parse.y +++ b/parse.y @@ -978,7 +978,7 @@ stmt : keyword_alias fitem {lex_state = EXPR_FNAME;} fitem { /*%%%*/ value_expr($3); - $1->nd_value = ($1->nd_head) ? NEW_TO_ARY($3) : NEW_ARRAY($3); + $1->nd_value = $3; $$ = $1; /*% $$ = dispatch2(massign, $1, $3);