From: Vincent Pit Date: Tue, 4 Jan 2011 18:10:07 +0000 (+0100) Subject: Always skip seen ops in our peep replacement X-Git-Tag: rt64435^0 X-Git-Url: http://git.vpit.fr/?p=perl%2Fmodules%2Fautovivification.git;a=commitdiff_plain;h=6bc99454a8284978b14854642fac0123c77bac45 Always skip seen ops in our peep replacement Sometimes, there are loops in the next chain as e.g. with infinite loops like "for (;;) { ... }". We can't reuse the op_opt member because it's almost always set after the first pass. This fixes RT #64435. --- diff --git a/MANIFEST b/MANIFEST index c1af216..8e98d27 100644 --- a/MANIFEST +++ b/MANIFEST @@ -22,6 +22,7 @@ t/34-array-numerous.t t/40-scope.t t/41-padsv.t t/42-deparse.t +t/43-peep.t t/50-threads.t t/51-threads-teardown.t t/91-pod.t diff --git a/autovivification.xs b/autovivification.xs index d07fd07..62162de 100644 --- a/autovivification.xs +++ b/autovivification.xs @@ -107,8 +107,6 @@ typedef struct { #endif /* A_WORKAROUND_REQUIRE_PROPAGATION */ -#if !A_HAS_RPEEP - #define PTABLE_NAME ptable_seen #define PTABLE_VAL_FREE(V) NOOP @@ -119,12 +117,6 @@ typedef struct { #define ptable_seen_clear(T) ptable_seen_clear(aPTBLMS_ (T)) #define ptable_seen_free(T) ptable_seen_free(aPTBLMS_ (T)) -#endif /* !A_HAS_RPEEP */ - -#define A_NEED_CXT ((A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION) || !A_HAS_RPEEP) - -#if A_NEED_CXT - #define MY_CXT_KEY __PACKAGE__ "::_guts" XS_VERSION typedef struct { @@ -132,9 +124,7 @@ typedef struct { ptable *tbl; /* It really is a ptable_hints */ tTHX owner; #endif /* A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION */ -#if !A_HAS_RPEEP ptable *seen; /* It really is a ptable_seen */ -#endif /* !A_HAS_RPEEP */ } my_cxt_t; START_MY_CXT @@ -190,15 +180,11 @@ STATIC void a_thread_cleanup(pTHX_ void *ud) { #if A_WORKAROUND_REQUIRE_PROPAGATION ptable_hints_free(MY_CXT.tbl); #endif /* A_WORKAROUND_REQUIRE_PROPAGATION */ -#if !A_HAS_RPEEP ptable_seen_free(MY_CXT.seen); -#endif /* !A_HAS_RPEEP */ } #endif /* A_THREADSAFE */ -#endif /* A_NEED_CXT */ - #if A_WORKAROUND_REQUIRE_PROPAGATION STATIC IV a_require_tag(pTHX) { @@ -947,33 +933,19 @@ STATIC OP *a_ck_root(pTHX_ OP *o) { STATIC peep_t a_old_peep = 0; /* This is actually the rpeep past 5.13.5 */ -#if !A_HAS_RPEEP -# define A_PEEP_REC_PROTO STATIC void a_peep_rec(pTHX_ OP *o, ptable *seen) -#else /* !A_HAS_RPEEP */ -# define A_PEEP_REC_PROTO STATIC void a_peep_rec(pTHX_ OP *o) -#endif /* A_HAS_RPEEP */ - -A_PEEP_REC_PROTO; -A_PEEP_REC_PROTO { -#if !A_HAS_RPEEP -# define a_peep_rec(O) a_peep_rec(aTHX_ (O), seen) -#else /* !A_HAS_RPEEP */ -# define a_peep_rec(O) a_peep_rec(aTHX_ (O)) -#endif /* A_HAS_RPEEP */ - dA_MAP_THX; - -#if !A_HAS_RPEEP - if (ptable_fetch(seen, o)) - return; -#endif +STATIC void a_peep_rec(pTHX_ OP *o, ptable *seen); +STATIC void a_peep_rec(pTHX_ OP *o, ptable *seen) { +#define a_peep_rec(O) a_peep_rec(aTHX_ (O), seen) for (; o; o = o->op_next) { + dA_MAP_THX; const a_op_info *oi = NULL; UV flags = 0; -#if !A_HAS_RPEEP + if (ptable_fetch(seen, o)) + break; ptable_seen_store(seen, o, o); -#endif + switch (o->op_type) { case OP_PADSV: if (o->op_ppaddr != a_pp_deref) { @@ -1058,15 +1030,14 @@ A_PEEP_REC_PROTO { } STATIC void a_peep(pTHX_ OP *o) { -#if !A_HAS_RPEEP dMY_CXT; ptable *seen = MY_CXT.seen; - ptable_seen_clear(seen); -#endif /* !A_HAS_RPEEP */ - a_old_peep(aTHX_ o); + + ptable_seen_clear(seen); a_peep_rec(o); + ptable_seen_clear(seen); } /* --- Interpreter setup/teardown ------------------------------------------ */ @@ -1083,17 +1054,13 @@ STATIC void a_teardown(pTHX_ void *root) { return; #endif -#if A_NEED_CXT { dMY_CXT; # if A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION ptable_hints_free(MY_CXT.tbl); # endif /* A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION */ -# if !A_HAS_RPEEP ptable_seen_free(MY_CXT.seen); -# endif /* !A_HAS_RPEEP */ } -#endif /* A_NEED_CXT */ PL_check[OP_PADANY] = MEMBER_TO_FPTR(a_old_ck_padany); a_old_ck_padany = 0; @@ -1141,18 +1108,14 @@ STATIC void a_setup(pTHX) { if (a_initialized) return; -#if A_NEED_CXT { MY_CXT_INIT; # if A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION MY_CXT.tbl = ptable_new(); MY_CXT.owner = aTHX; # endif /* A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION */ -# if !A_HAS_RPEEP MY_CXT.seen = ptable_new(); -# endif /* !A_RPEEP */ } -#endif /* A_NEED_CXT */ a_old_ck_padany = PL_check[OP_PADANY]; PL_check[OP_PADANY] = MEMBER_TO_FPTR(a_ck_padany); @@ -1237,7 +1200,7 @@ BOOT: a_setup(); } -#if A_THREADSAFE && (A_WORKAROUND_REQUIRE_PROPAGATION || !A_HAS_RPEEP) +#if A_THREADSAFE void CLONE(...) @@ -1246,9 +1209,7 @@ PREINIT: #if A_WORKAROUND_REQUIRE_PROPAGATION ptable *t; #endif -#if !A_HAS_RPEEP ptable *s; -#endif PPCODE: { dMY_CXT; @@ -1262,9 +1223,7 @@ PPCODE: a_ptable_clone_ud_deinit(ud); } #endif -#if !A_HAS_RPEEP s = ptable_new(); -#endif } { MY_CXT_CLONE; @@ -1272,14 +1231,12 @@ PPCODE: MY_CXT.tbl = t; MY_CXT.owner = aTHX; #endif -#if !A_HAS_RPEEP MY_CXT.seen = s; -#endif } reap(3, a_thread_cleanup, NULL); XSRETURN(0); -#endif +#endif /* A_THREADSAFE */ SV * _tag(SV *hint) diff --git a/t/43-peep.t b/t/43-peep.t new file mode 100644 index 0000000..5050821 --- /dev/null +++ b/t/43-peep.t @@ -0,0 +1,198 @@ +#!perl -T + +use strict; +use warnings; + +use Test::More tests => 11 + 6 * 3; + +{ + my $desc = 'peephole optimization of conditionals'; + my $x; + + local $@; + my $code = eval <<' TESTCASE'; + no autovivification; + sub { + if ($_[0]) { + my $z = $x->{a}; + return 1; + } elsif ($_[1] || $_[2]) { + my $z = $x->{b}; + return 2; + } elsif ($_[3] && $_[4]) { + my $z = $x->{c}; + return 3; + } elsif ($_[5] ? $_[6] : 0) { + my $z = $x->{d}; + return 4; + } else { + my $z = $x->{e}; + return 5; + } + return 0; + } + TESTCASE + is $@, '', "$desc compiled fine"; + + my $ret = $code->(1); + is_deeply $x, undef, "$desc : first branch did not autovivify"; + is $ret, 1, "$desc : first branch returned 1"; + + $ret = $code->(0, 1); + is_deeply $x, undef, "$desc : second branch did not autovivify"; + is $ret, 2, "$desc : second branch returned 2"; + + $ret = $code->(0, 0, 0, 1, 1); + is_deeply $x, undef, "$desc : third branch did not autovivify"; + is $ret, 3, "$desc : third branch returned 3"; + + $ret = $code->(0, 0, 0, 0, 0, 1, 1); + is_deeply $x, undef, "$desc : fourth branch did not autovivify"; + is $ret, 4, "$desc : fourth branch returned 4"; + + $ret = $code->(); + is_deeply $x, undef, "$desc : fifth branch did not autovivify"; + is $ret, 5, "$desc : fifth branch returned 5"; +} + +{ + my $desc = 'peephole optimization of C-style loops'; + my $x; + + local $@; + my $code = eval <<' TESTCASE'; + no autovivification; + sub { + my $ret = 0; + for ( + my ($z, $i) = ($x->[100], 0) + ; + do { my $z = $x->[200]; $i < 4 } + ; + do { my $z = $x->[300]; ++$i } + ) { + my $z = $x->[$i]; + $ret += $i; + } + return $ret; + } + TESTCASE + is $@, '', "$desc compiled fine"; + + my $ret = $code->(); + is_deeply $x, undef, "$desc did not autovivify"; + is $ret, 6, "$desc returned 0+1+2+3"; +} + +{ + my $desc = 'peephole optimization of range loops'; + my $x; + + local $@; + my $code = eval <<' TESTCASE'; + no autovivification; + sub { + my $ret = 0; + for ((do { my $z = $x->[100]; 0 }) .. (do { my $z = $x->[200]; 3 })) { + my $z = $x->[$_]; + $ret += $_; + } + return $ret; + } + TESTCASE + is $@, '', "$desc compiled fine"; + + my $ret = $code->(); + is_deeply $x, undef, "$desc did not autovivify"; + is $ret, 6, "$desc returned 0+1+2+3"; +} + +{ + my $desc = 'peephole optimization of empty loops (RT #64435)'; + my $x; + + local $@; + my $code = eval <<' TESTCASE'; + no autovivification; + sub { + my $ret = 0; + for (;;) { + ++$ret; + return $ret; + } + return $ret; + } + TESTCASE + is $@, '', "$desc compiled fine"; + + my $ret = $code->(); + is_deeply $x, undef, "$desc did not autovivify"; + is $ret, 1, "$desc returned 1"; +} + +{ + my $desc = 'peephole optimization of map'; + my $x; + + local $@; + my $code = eval <<' TESTCASE'; + no autovivification; + sub { + join ':', map { + my $z = $x->[$_]; + "x${_}y" + } @_ + } + TESTCASE + is $@, '', "$desc compiled fine"; + + my $ret = $code->(1, 2); + is_deeply $x, undef, "$desc did not autovivify"; + is $ret, 'x1y:x2y', "$desc returned the right value"; +} + +{ + my $desc = 'peephole optimization of grep'; + my $x; + + local $@; + my $code = eval <<' TESTCASE'; + no autovivification; + sub { + join ':', grep { + my $z = $x->[$_]; + $_ <= 3 + } @_ + } + TESTCASE + is $@, '', "$desc compiled fine"; + + my $ret = $code->(1 .. 5); + is_deeply $x, undef, "$desc did not autovivify"; + is $ret, '1:2:3', "$desc returned the right value"; +} + +{ + my $desc = 'peephole optimization of substitutions'; + my $x; + + local $@; + my $code = eval <<' TESTCASE'; + no autovivification; + sub { + my $str = $_[0]; + $str =~ s{ + ([0-9]) + }{ + my $z = $x->[$1]; + 9 - $1; + }xge; + $str; + } + TESTCASE + is $@, '', "$desc compiled fine"; + + my $ret = $code->('0123456789'); + is_deeply $x, undef, "$desc did not autovivify"; + is $ret, '9876543210', "$desc returned the right value"; +}