don't set SvPADTMP() on PADGV's

Under threaded builds, GVs for OPs are stored in the pad rather than
being directly attached to the op. For some reason, all such GV's were
getting the SvPADTMP flag set. There seems to be be no good reason for
this, and after skipping setting the flag, all tests still pass.

The advantage of not setting this flag is that there are quite a few
hot places in the code that do

    if (SvPADTMP(sv) && !IS_PADGV(sv)) {
        ...

I've replaced them all with

    if (SvPADTMP(sv)) {
        assert(!IS_PADGV(sv));
        ...

Since the IS_PADGV() macro expands to something quite heavyweight, this is
quite a saving: for example this commit reduces the size of pp_entersub by
111 bytes.

diff --git a/op.c b/op.c
index 45a8a37..8515800 100644 (file)
--- a/op.c
+++ b/op.c
@@ -5192,7 +5192,6 @@ Perl_newPADOP(pTHX_ I32 type, I32 flags, SV *sv)
     SvREFCNT_dec(PAD_SVl(padop->op_padix));
     PAD_SETSV(padop->op_padix, sv);
     assert(sv);
-    SvPADTMP_on(sv);
     padop->op_next = (OP*)padop;
     padop->op_flags = (U8)flags;
     if (PL_opargs[type] & OA_RETSCALAR)

diff --git a/pp.c b/pp.c
index 1674ae6..4ec6887 100644 (file)
--- a/pp.c
+++ b/pp.c
@@ -571,8 +571,10 @@ S_refto(pTHX_ SV *sv)
        SvTEMP_off(sv);
        SvREFCNT_inc_void_NN(sv);
     }
-    else if (SvPADTMP(sv) && !IS_PADGV(sv))
+    else if (SvPADTMP(sv)) {
+        assert(!IS_PADGV(sv));
         sv = newSVsv(sv);
+    }
     else {
        SvTEMP_off(sv);
        SvREFCNT_inc_void_NN(sv);
@@ -1707,10 +1709,11 @@ PP(pp_repeat)
                    SvREADONLY_on(*SP);
                }
 #else
-               if (*SP)
-                {
-                   if (mod && SvPADTMP(*SP) && !IS_PADGV(*SP))
+                if (*SP) {
+                   if (mod && SvPADTMP(*SP)) {
+                       assert(!IS_PADGV(*SP));
                        *SP = sv_mortalcopy(*SP);
+                   }
                   SvTEMP_off((*SP));
                }
 #endif
@@ -4896,8 +4899,10 @@ PP(pp_lslice)
            is_something_there = TRUE;
            if (!(*lelem = firstrelem[ix]))
                *lelem = &PL_sv_undef;
-           else if (mod && SvPADTMP(*lelem) && !IS_PADGV(*lelem))
+           else if (mod && SvPADTMP(*lelem)) {
+                assert(!IS_PADGV(*lelem));
                *lelem = firstrelem[ix] = sv_mortalcopy(*lelem);
+            }
        }
     }
     if (is_something_there)

diff --git a/pp_ctl.c b/pp_ctl.c
index 43466fe..7b516da 100644 (file)
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -938,7 +938,8 @@ PP(pp_grepstart)
     SAVEVPTR(PL_curpm);
 
     src = PL_stack_base[*PL_markstack_ptr];
-    if (SvPADTMP(src) && !IS_PADGV(src)) {
+    if (SvPADTMP(src)) {
+        assert(!IS_PADGV(src));
        src = PL_stack_base[*PL_markstack_ptr] = sv_mortalcopy(src);
        PL_tmps_floor++;
     }
@@ -1090,7 +1091,10 @@ PP(pp_mapwhile)
 
        /* set $_ to the new source item */
        src = PL_stack_base[PL_markstack_ptr[-1]];
-       if (SvPADTMP(src) && !IS_PADGV(src)) src = sv_mortalcopy(src);
+       if (SvPADTMP(src)) {
+            assert(!IS_PADGV(src));
+            src = sv_mortalcopy(src);
+        }
        SvTEMP_off(src);
        if (PL_op->op_private & OPpGREP_LEX)
            PAD_SVl(PL_op->op_targ) = src;

diff --git a/pp_hot.c b/pp_hot.c
index 552be61..ae88d83 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -1916,8 +1916,10 @@ PP(pp_iter)
                 *itersvp = NULL;
                 Perl_croak(aTHX_ "Use of freed value in iteration");
             }
-            if (SvPADTMP(sv) && !IS_PADGV(sv))
+            if (SvPADTMP(sv)) {
+                assert(!IS_PADGV(sv));
                 sv = newSVsv(sv);
+            }
             else {
                 SvTEMP_off(sv);
                 SvREFCNT_inc_simple_void_NN(sv);
@@ -2432,7 +2434,8 @@ PP(pp_grepwhile)
        SAVEVPTR(PL_curpm);
 
        src = PL_stack_base[*PL_markstack_ptr];
-       if (SvPADTMP(src) && !IS_PADGV(src)) {
+       if (SvPADTMP(src)) {
+            assert(!IS_PADGV(src));
            src = PL_stack_base[*PL_markstack_ptr] = sv_mortalcopy(src);
            PL_tmps_floor++;
        }
@@ -2693,8 +2696,10 @@ try_autoload:
            while (items--) {
                if (*MARK)
                {
-                   if (SvPADTMP(*MARK) && !IS_PADGV(*MARK))
+                   if (SvPADTMP(*MARK)) {
+                        assert(!IS_PADGV(*MARK));
                        *MARK = sv_mortalcopy(*MARK);
+                    }
                    SvTEMP_off(*MARK);
                }
                MARK++;
@@ -2760,8 +2765,10 @@ try_autoload:
            SSize_t items = SP - mark;
            while (items--) {
                mark++;
-               if (*mark && SvPADTMP(*mark) && !IS_PADGV(*mark))
+               if (*mark && SvPADTMP(*mark)) {
+                    assert(!IS_PADGV(*mark));
                    *mark = sv_mortalcopy(*mark);
+                }
            }
        }
        /* We assume first XSUB in &DB::sub is the called one. */

diff --git a/pp_sort.c b/pp_sort.c
index ae0c9c1..4741d71 100644 (file)
--- a/pp_sort.c
+++ b/pp_sort.c
@@ -1609,8 +1609,10 @@ PP(pp_sort)
     copytmps = !sorting_av && PL_sortcop;
     for (i=max; i > 0 ; i--) {
        if ((*p1 = *p2++)) {                    /* Weed out nulls. */
-           if (copytmps && SvPADTMP(*p1) && !IS_PADGV(*p1))
+           if (copytmps && SvPADTMP(*p1)) {
+                assert(!IS_PADGV(*p1));
                *p1 = sv_mortalcopy(*p1);
+            }
            SvTEMP_off(*p1);
            if (!PL_sortcop) {
                if (priv & OPpSORT_NUMERIC) {

diff --git a/regexec.c b/regexec.c
index 6dd0297..df27193 100644 (file)
--- a/regexec.c
+++ b/regexec.c
@@ -2528,13 +2528,14 @@ Perl_regexec_flags(pTHX_ REGEXP * const rx, char *stringarg, char *strend,
     /* see how far we have to get to not match where we matched before */
     reginfo->till = stringarg + minend;
 
-    if (prog->extflags & RXf_EVAL_SEEN && SvPADTMP(sv) && !IS_PADGV(sv)) {
+    if (prog->extflags & RXf_EVAL_SEEN && SvPADTMP(sv)) {
         /* SAVEFREESV, not sv_mortalcopy, as this SV must last until after
            S_cleanup_regmatch_info_aux has executed (registered by
            SAVEDESTRUCTOR_X below).  S_cleanup_regmatch_info_aux modifies
            magic belonging to this SV.
            Not newSVsv, either, as it does not COW.
         */
+        assert(!IS_PADGV(sv));
         reginfo->sv = newSV(0);
         SvSetSV_nosteal(reginfo->sv, sv);
         SAVEFREESV(reginfo->sv);