This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Simplify double-nextstate optimisation
authorFather Chrysostomos <sprout@cpan.org>
Tue, 14 Oct 2014 03:37:29 +0000 (20:37 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Wed, 15 Oct 2014 05:01:46 +0000 (22:01 -0700)
Executing two nextstate ops in a row is equivalent to executing just
the second, so the peephole optimiser eliminates one of them.

The double-nextstate optimisation seems to have been written under
the assumption that the current op cannot be elided from the op_next
change, which is untrue, as we have oldop (the previous op in
the chain).

If the current op and the one following it are both nextstate ops,
then the contents of the second are moved over to the first and the
second is nulled.  Then current_op->op_next is adjusted to skip over
the newly-nulled op.

The the next op in the chain, the one that the first nextstate op now
points to, is processed.

The result is that every other nextstate in sequence of three or more
would be elided:

1        <;> nextstate(main 1 -e:1) v ->2
-        <0> ex-const v ->-
-        <0> null v ->-
-        <0> ex-const v ->2
2        <;> nextstate(main 1 -e:1) v:{ ->3
-        <0> ex-const v ->-
-        <0> null v ->-
-        <0> ex-const v ->3
3        <;> nextstate(main 1 -e:1) v:{ ->4
-        <0> ex-const v ->-
-        <0> null v ->-
-        <0> ex-const v ->4
4        <;> nextstate(main 1 -e:1) v:{ ->5

We don’t need to go through the complication of making the first next-
state equivalent to the second.  Just null the first one and adjust
the previous op’s pointer (oldop->op_next) to point to the second one.

Now all consecutive nextstates get nulled except the last:

-        <0> ex-nextstate v ->-
-        <0> ex-const v ->-
-        <0> ex-nextstate v ->-
-        <0> ex-const v ->-
-        <0> ex-nextstate v ->-
-        <0> ex-const v ->-
-        <0> ex-nextstate v ->-
-        <0> ex-const v ->-
-        <0> ex-nextstate v ->1
-        <0> ex-const v ->-
-        <0> ex-nextstate v ->1
-        <0> ex-const v ->1
1        <;> nextstate(main 1 -e:1) v:{ ->2

The only visible difference this makes is in B::Deparse output, but
this changed in 5.14 when the optimisation was first introduced, so I
think changing it again is acceptable:

$ perl5.12.4 -MO=Deparse -e 'use strict; 0; use warnings; 0'
use strict 'refs';
'???';
use warnings;
'???';
-e syntax OK

$ perl5.14.4 -MO=Deparse -e 'use strict; 0; use warnings; 0'
use warnings;
use strict 'refs';
'???';
;
'???';
-e syntax OK

$ ./perl -Ilib -MO=Deparse -e 'use strict; 0; use warnings; 0'
;
'???';
use warnings;
use strict;
'???';
-e syntax OK

ext/B/t/optree_specials.t
lib/B/Deparse.t
op.c
t/op/opt.t

index f22b77f..652b5ab 100644 (file)
@@ -45,9 +45,9 @@ checkOptree ( name    => 'BEGIN',
 # 1        <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$ ->2
 # 3        <1> require sK/1 ->4
 # 2           <$> const[PV "strict.pm"] s/BARE ->3
-# 4        <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$ ->5
+# -        <0> ex-nextstate v ->4
 # -        <@> lineseq K ->-
-# -           <0> null ->5
+# 4           <;> nextstate(B::Concise -275 Concise.pm:356) :*,&,{,x*,x&,x$,$ ->5
 # 9           <1> entersub[t1] KS*/TARG,STRICT ->a
 # 5              <0> pushmark s ->6
 # 6              <$> const[PV "strict"] sM ->7
@@ -59,9 +59,9 @@ checkOptree ( name    => 'BEGIN',
 # b        <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$ ->c
 # d        <1> require sK/1 ->e
 # c           <$> const[PV "strict.pm"] s/BARE ->d
-# e        <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$ ->f
+# -        <0> ex-nextstate v ->e
 # -        <@> lineseq K ->-
-# -           <0> null ->f
+# e           <;> nextstate(B::Concise -265 Concise.pm:367) :*,&,x*,x&,x$,$ ->f
 # j           <1> entersub[t1] KS*/TARG,STRICT ->k
 # f              <0> pushmark s ->g
 # g              <$> const[PV "strict"] sM ->h
@@ -73,9 +73,9 @@ checkOptree ( name    => 'BEGIN',
 # l        <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$ ->m
 # n        <1> require sK/1 ->o
 # m           <$> const[PV "warnings.pm"] s/BARE ->n
-# o        <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$ ->p
+# -        <0> ex-nextstate v ->o
 # -        <@> lineseq K ->-
-# -           <0> null ->p
+# o           <;> nextstate(B::Concise -254 Concise.pm:386) :*,&,{,x*,x&,x$,$ ->p
 # t           <1> entersub[t1] KS*/TARG,STRICT ->u
 # p              <0> pushmark s ->q
 # q              <$> const[PV "warnings"] sM ->r
@@ -95,9 +95,9 @@ EOT_EOT
 # 1        <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$ ->2
 # 3        <1> require sK/1 ->4
 # 2           <$> const(PV "strict.pm") s/BARE ->3
-# 4        <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$ ->5
+# -        <0> ex-nextstate v ->4
 # -        <@> lineseq K ->-
-# -           <0> null ->5
+# 4           <;> nextstate(B::Concise -275 Concise.pm:356) :*,&,{,x*,x&,x$,$ ->5
 # 9           <1> entersub[t1] KS*/TARG,STRICT ->a
 # 5              <0> pushmark s ->6
 # 6              <$> const(PV "strict") sM ->7
@@ -109,9 +109,9 @@ EOT_EOT
 # b        <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$ ->c
 # d        <1> require sK/1 ->e
 # c           <$> const(PV "strict.pm") s/BARE ->d
-# e        <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$ ->f
+# -        <0> ex-nextstate v ->e
 # -        <@> lineseq K ->-
-# -           <0> null ->f
+# e           <;> nextstate(B::Concise -265 Concise.pm:367) :*,&,x*,x&,x$,$ ->f
 # j           <1> entersub[t1] KS*/TARG,STRICT ->k
 # f              <0> pushmark s ->g
 # g              <$> const(PV "strict") sM ->h
@@ -123,9 +123,9 @@ EOT_EOT
 # l        <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$ ->m
 # n        <1> require sK/1 ->o
 # m           <$> const(PV "warnings.pm") s/BARE ->n
-# o        <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$ ->p
+# -        <0> ex-nextstate v ->o
 # -        <@> lineseq K ->-
-# -           <0> null ->p
+# o           <;> nextstate(B::Concise -254 Concise.pm:386) :*,&,{,x*,x&,x$,$ ->p
 # t           <1> entersub[t1] KS*/TARG,STRICT ->u
 # p              <0> pushmark s ->q
 # q              <$> const(PV "warnings") sM ->r
@@ -241,7 +241,7 @@ checkOptree ( name  => 'all of BEGIN END INIT CHECK UNITCHECK -exec',
 # 1  <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$
 # 2  <$> const[PV "strict.pm"] s/BARE
 # 3  <1> require sK/1
-# 4  <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$
+# 4  <;> nextstate(B::Concise -275 Concise.pm:356) :*,&,{,x*,x&,x$,$
 # 5  <0> pushmark s
 # 6  <$> const[PV "strict"] sM
 # 7  <$> const[PV "refs"] sM
@@ -252,7 +252,7 @@ checkOptree ( name  => 'all of BEGIN END INIT CHECK UNITCHECK -exec',
 # b  <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$
 # c  <$> const[PV "strict.pm"] s/BARE
 # d  <1> require sK/1
-# e  <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$
+# e  <;> nextstate(B::Concise -265 Concise.pm:367) :*,&,x*,x&,x$,$
 # f  <0> pushmark s
 # g  <$> const[PV "strict"] sM
 # h  <$> const[PV "refs"] sM
@@ -263,7 +263,7 @@ checkOptree ( name  => 'all of BEGIN END INIT CHECK UNITCHECK -exec',
 # l  <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$
 # m  <$> const[PV "warnings.pm"] s/BARE
 # n  <1> require sK/1
-# o  <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$
+# o  <;> nextstate(B::Concise -254 Concise.pm:386) :*,&,{,x*,x&,x$,$
 # p  <0> pushmark s
 # q  <$> const[PV "warnings"] sM
 # r  <$> const[PV "qw"] sM
@@ -300,7 +300,7 @@ EOT_EOT
 # 1  <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$
 # 2  <$> const(PV "strict.pm") s/BARE
 # 3  <1> require sK/1
-# 4  <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$
+# 4  <;> nextstate(B::Concise -275 Concise.pm:356) :*,&,{,x*,x&,x$,$
 # 5  <0> pushmark s
 # 6  <$> const(PV "strict") sM
 # 7  <$> const(PV "refs") sM
@@ -311,7 +311,7 @@ EOT_EOT
 # b  <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$
 # c  <$> const(PV "strict.pm") s/BARE
 # d  <1> require sK/1
-# e  <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$
+# e  <;> nextstate(B::Concise -265 Concise.pm:367) :*,&,x*,x&,x$,$
 # f  <0> pushmark s
 # g  <$> const(PV "strict") sM
 # h  <$> const(PV "refs") sM
@@ -322,7 +322,7 @@ EOT_EOT
 # l  <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$
 # m  <$> const(PV "warnings.pm") s/BARE
 # n  <1> require sK/1
-# o  <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$
+# o  <;> nextstate(B::Concise -254 Concise.pm:386) :*,&,{,x*,x&,x$,$
 # p  <0> pushmark s
 # q  <$> const(PV "warnings") sM
 # r  <$> const(PV "qw") sM
@@ -369,7 +369,7 @@ checkOptree ( name  => 'regression test for patch 25352',
 # 1  <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$
 # 2  <$> const[PV "strict.pm"] s/BARE
 # 3  <1> require sK/1
-# 4  <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$
+# 4  <;> nextstate(B::Concise -275 Concise.pm:356) :*,&,{,x*,x&,x$,$
 # 5  <0> pushmark s
 # 6  <$> const[PV "strict"] sM
 # 7  <$> const[PV "refs"] sM
@@ -380,7 +380,7 @@ checkOptree ( name  => 'regression test for patch 25352',
 # b  <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$
 # c  <$> const[PV "strict.pm"] s/BARE
 # d  <1> require sK/1
-# e  <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$
+# e  <;> nextstate(B::Concise -265 Concise.pm:367) :*,&,x*,x&,x$,$
 # f  <0> pushmark s
 # g  <$> const[PV "strict"] sM
 # h  <$> const[PV "refs"] sM
@@ -391,7 +391,7 @@ checkOptree ( name  => 'regression test for patch 25352',
 # l  <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$
 # m  <$> const[PV "warnings.pm"] s/BARE
 # n  <1> require sK/1
-# o  <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$
+# o  <;> nextstate(B::Concise -254 Concise.pm:386) :*,&,{,x*,x&,x$,$
 # p  <0> pushmark s
 # q  <$> const[PV "warnings"] sM
 # r  <$> const[PV "qw"] sM
@@ -403,7 +403,7 @@ EOT_EOT
 # 1  <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$
 # 2  <$> const(PV "strict.pm") s/BARE
 # 3  <1> require sK/1
-# 4  <;> nextstate(B::Concise -275 Concise.pm:356) v:*,&,{,x*,x&,x$,$
+# 4  <;> nextstate(B::Concise -275 Concise.pm:356) :*,&,{,x*,x&,x$,$
 # 5  <0> pushmark s
 # 6  <$> const(PV "strict") sM
 # 7  <$> const(PV "refs") sM
@@ -414,7 +414,7 @@ EOT_EOT
 # b  <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$
 # c  <$> const(PV "strict.pm") s/BARE
 # d  <1> require sK/1
-# e  <;> nextstate(B::Concise -265 Concise.pm:367) v:*,&,x*,x&,x$,$
+# e  <;> nextstate(B::Concise -265 Concise.pm:367) :*,&,x*,x&,x$,$
 # f  <0> pushmark s
 # g  <$> const(PV "strict") sM
 # h  <$> const(PV "refs") sM
@@ -425,7 +425,7 @@ EOT_EOT
 # l  <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$
 # m  <$> const(PV "warnings.pm") s/BARE
 # n  <1> require sK/1
-# o  <;> nextstate(B::Concise -254 Concise.pm:386) v:*,&,{,x*,x&,x$,$
+# o  <;> nextstate(B::Concise -254 Concise.pm:386) :*,&,{,x*,x&,x$,$
 # p  <0> pushmark s
 # q  <$> const(PV "warnings") sM
 # r  <$> const(PV "qw") sM
index 0046ce1..8a7a425 100644 (file)
@@ -338,8 +338,8 @@ tr/\x{345}/\x{370}/;
 1;
 ####
 # Constants in a block
+# CONTEXT no warnings;
 {
-    no warnings;
     '???';
     2;
 }
diff --git a/op.c b/op.c
index 50346bf..2217307 100644 (file)
--- a/op.c
+++ b/op.c
@@ -11881,44 +11881,9 @@ Perl_rpeep(pTHX_ OP *o)
                    nextop = nextop->op_next;
 
                if (nextop && (nextop->op_type == OP_NEXTSTATE)) {
-                   COP *firstcop = (COP *)o;
-                   COP *secondcop = (COP *)nextop;
-                   /* We want the COP pointed to by o (and anything else) to
-                      become the next COP down the line.  */
-                   cop_free(firstcop);
-
-                   firstcop->op_next = secondcop->op_next;
-
-                   /* Now steal all its pointers, and duplicate the other
-                      data.  */
-                   firstcop->cop_line = secondcop->cop_line;
-#ifdef USE_ITHREADS
-                   firstcop->cop_stashoff = secondcop->cop_stashoff;
-                   firstcop->cop_file = secondcop->cop_file;
-#else
-                   firstcop->cop_stash = secondcop->cop_stash;
-                   firstcop->cop_filegv = secondcop->cop_filegv;
-#endif
-                   firstcop->cop_hints = secondcop->cop_hints;
-                   firstcop->cop_seq = secondcop->cop_seq;
-                   firstcop->cop_warnings = secondcop->cop_warnings;
-                   firstcop->cop_hints_hash = secondcop->cop_hints_hash;
-
-#ifdef USE_ITHREADS
-                   secondcop->cop_stashoff = 0;
-                   secondcop->cop_file = NULL;
-#else
-                   secondcop->cop_stash = NULL;
-                   secondcop->cop_filegv = NULL;
-#endif
-                   secondcop->cop_warnings = NULL;
-                   secondcop->cop_hints_hash = NULL;
-
-                   /* If we use op_null(), and hence leave an ex-COP, some
-                      warnings are misreported. For example, the compile-time
-                      error in 'use strict; no strict refs;'  */
-                   secondcop->op_type = OP_NULL;
-                   secondcop->op_ppaddr = PL_ppaddr[OP_NULL];
+                   op_null(o);
+                   if (oldop)
+                       oldop->op_next = nextop;
                }
            }
            break;
index 71eedcf..aa73fd7 100644 (file)
@@ -9,7 +9,7 @@ BEGIN {
     @INC = '../lib';
 }
 
-plan 19;
+plan 20;
 
 use B qw 'svref_2object OPpASSIGN_COMMON';
 
@@ -56,6 +56,12 @@ for (['CONSTANT', sub {          join "foo", "bar"    }, 0, "bar"    ],
 }
 
 
+# nextstate multiple times becoming one nextstate
+
+is svref_2object(sub { 0;0;0;0;0;0;time })->START->next->name, 'time',
+  'multiple nextstates become one';
+
+
 # split to array
 
 for(['@pkgary'      , '@_'       ],