Add SS_ADD_* macros and replace most SSPUSH* uses

The current idiom for adding an entry to the savestack is

    SSCHECK(3);
    SSPUSHINT(...);
    SSPUSHPTR(...);
    SSPUSHUV(SAVEt_...);

Replace this with a new idiom:

    {
        dSS_ADD;
        SS_ADD_INT(...);
        SS_ADD_PTR(...);
        SS_ADD_UV(SAVEt_...);
        SS_ADD_END(3);
    }

This is designed to be more efficient.

First, it defines some local vars, and defers updating PL_savestack_ix
to the end.

Second, it performs the 'is there space on the stack' check *after*
pushing. Doing the check last means that values in registers will have
been pushed and no longer needed, so don't need saving around the call to
grow. Also, tail-call elimination of the grow() can be done. These changes
reduce the code of something like save_pushptrptr() to half its former
size.

Of course, doing the size check *after* pushing means we must always
ensure there are SS_MAXPUSH free slots on the savestack */

diff --git a/pp_hot.c b/pp_hot.c
index 333c593..599b759 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -376,8 +376,11 @@ PP(pp_padrange)
                     | SAVEt_CLEARPADRANGE);
         assert(OPpPADRANGE_COUNTMASK + 1 == (1 <<OPpPADRANGE_COUNTSHIFT));
         assert((payload >> (OPpPADRANGE_COUNTSHIFT+SAVE_TIGHT_SHIFT)) == base);
-        SSCHECK(1);
-        SSPUSHUV(payload);
+        {
+            dSS_ADD;
+            SS_ADD_UV(payload);
+            SS_ADD_END(1);
+        }
 
         for (i = 0; i <count; i++)
             SvPADSTALE_off(*svp++); /* mark lexical as active */

diff --git a/scope.c b/scope.c
index 3d50932..aad2e24 100644 (file)
--- a/scope.c
+++ b/scope.c
@@ -199,10 +199,11 @@ void
 Perl_save_pushptrptr(pTHX_ void *const ptr1, void *const ptr2, const int type)
 {
     dVAR;
-    SSCHECK(3);
-    SSPUSHPTR(ptr1);
-    SSPUSHPTR(ptr2);
-    SSPUSHUV(type);
+    dSS_ADD;
+    SS_ADD_PTR(ptr1);
+    SS_ADD_PTR(ptr2);
+    SS_ADD_UV(type);
+    SS_ADD_END(3);
 }
 
 SV *
@@ -264,14 +265,15 @@ void
 Perl_save_set_svflags(pTHX_ SV* sv, U32 mask, U32 val)
 {
     dVAR;
+    dSS_ADD;
 
     PERL_ARGS_ASSERT_SAVE_SET_SVFLAGS;
 
-    SSCHECK(4);
-    SSPUSHPTR(sv);
-    SSPUSHINT(mask);
-    SSPUSHINT(val);
-    SSPUSHUV(SAVEt_SET_SVFLAGS);
+    SS_ADD_PTR(sv);
+    SS_ADD_INT(mask);
+    SS_ADD_INT(val);
+    SS_ADD_UV(SAVEt_SET_SVFLAGS);
+    SS_ADD_END(4);
 }
 
 void
@@ -364,22 +366,25 @@ void
 Perl_save_bool(pTHX_ bool *boolp)
 {
     dVAR;
+    dSS_ADD;
 
     PERL_ARGS_ASSERT_SAVE_BOOL;
 
-    SSCHECK(2);
-    SSPUSHPTR(boolp);
-    SSPUSHUV(SAVEt_BOOL | (*boolp << 8));
+    SS_ADD_PTR(boolp);
+    SS_ADD_UV(SAVEt_BOOL | (*boolp << 8));
+    SS_ADD_END(2);
 }
 
 void
 Perl_save_pushi32ptr(pTHX_ const I32 i, void *const ptr, const int type)
 {
     dVAR;
-    SSCHECK(3);
-    SSPUSHINT(i);
-    SSPUSHPTR(ptr);
-    SSPUSHUV(type);
+    dSS_ADD;
+
+    SS_ADD_INT(i);
+    SS_ADD_PTR(ptr);
+    SS_ADD_UV(type);
+    SS_ADD_END(3);
 }
 
 void
@@ -391,9 +396,10 @@ Perl_save_int(pTHX_ int *intp)
     PERL_ARGS_ASSERT_SAVE_INT;
 
     if ((int)(shifted >> SAVE_TIGHT_SHIFT) == *intp) {
-       SSCHECK(2);
-       SSPUSHPTR(intp);
-       SSPUSHUV(SAVEt_INT_SMALL | shifted);
+        dSS_ADD;
+       SS_ADD_PTR(intp);
+       SS_ADD_UV(SAVEt_INT_SMALL | shifted);
+        SS_ADD_END(2);
     } else
        save_pushi32ptr(*intp, intp, SAVEt_INT);
 }
@@ -402,24 +408,26 @@ void
 Perl_save_I8(pTHX_ I8 *bytep)
 {
     dVAR;
+    dSS_ADD;
 
     PERL_ARGS_ASSERT_SAVE_I8;
 
-    SSCHECK(2);
-    SSPUSHPTR(bytep);
-    SSPUSHUV(SAVEt_I8 | ((UV)*bytep << 8));
+    SS_ADD_PTR(bytep);
+    SS_ADD_UV(SAVEt_I8 | ((UV)*bytep << 8));
+    SS_ADD_END(2);
 }
 
 void
 Perl_save_I16(pTHX_ I16 *intp)
 {
     dVAR;
+    dSS_ADD;
 
     PERL_ARGS_ASSERT_SAVE_I16;
 
-    SSCHECK(2);
-    SSPUSHPTR(intp);
-    SSPUSHUV(SAVEt_I16 | ((UV)*intp << 8));
+    SS_ADD_PTR(intp);
+    SS_ADD_UV(SAVEt_I16 | ((UV)*intp << 8));
+    SS_ADD_END(2);
 }
 
 void
@@ -431,9 +439,10 @@ Perl_save_I32(pTHX_ I32 *intp)
     PERL_ARGS_ASSERT_SAVE_I32;
 
     if ((I32)(shifted >> SAVE_TIGHT_SHIFT) == *intp) {
-       SSCHECK(2);
-       SSPUSHPTR(intp);
-       SSPUSHUV(SAVEt_I32_SMALL | shifted);
+        dSS_ADD;
+       SS_ADD_PTR(intp);
+       SS_ADD_UV(SAVEt_I32_SMALL | shifted);
+        SS_ADD_END(2);
     } else
        save_pushi32ptr(*intp, intp, SAVEt_I32);
 }
@@ -475,12 +484,14 @@ void
 Perl_save_padsv_and_mortalize(pTHX_ PADOFFSET off)
 {
     dVAR;
-    SSCHECK(4);
+    dSS_ADD;
+
     ASSERT_CURPAD_ACTIVE("save_padsv");
-    SSPUSHPTR(SvREFCNT_inc_simple_NN(PL_curpad[off]));
-    SSPUSHPTR(PL_comppad);
-    SSPUSHLONG((long)off);
-    SSPUSHUV(SAVEt_PADSV_AND_MORTALIZE);
+    SS_ADD_PTR(SvREFCNT_inc_simple_NN(PL_curpad[off]));
+    SS_ADD_PTR(PL_comppad);
+    SS_ADD_LONG((long)off);
+    SS_ADD_UV(SAVEt_PADSV_AND_MORTALIZE);
+    SS_ADD_END(4);
 }
 
 void
@@ -507,9 +518,10 @@ void
 Perl_save_pushptr(pTHX_ void *const ptr, const int type)
 {
     dVAR;
-    SSCHECK(2);
-    SSPUSHPTR(ptr);
-    SSPUSHUV(type);
+    dSS_ADD;
+    SS_ADD_PTR(ptr);
+    SS_ADD_UV(type);
+    SS_ADD_END(2);
 }
 
 void
@@ -523,12 +535,16 @@ Perl_save_clearsv(pTHX_ SV **svp)
 
     ASSERT_CURPAD_ACTIVE("save_clearsv");
     SvPADSTALE_off(*svp); /* mark lexical as active */
-    if ((offset_shifted >> SAVE_TIGHT_SHIFT) != offset)
+    if ((offset_shifted >> SAVE_TIGHT_SHIFT) != offset) {
        Perl_croak(aTHX_ "panic: pad offset %"UVuf" out of range (%p-%p)",
                   offset, svp, PL_curpad);
+    }
 
-    SSCHECK(1);
-    SSPUSHUV(offset_shifted | SAVEt_CLEARSV);
+    {
+        dSS_ADD;
+        SS_ADD_UV(offset_shifted | SAVEt_CLEARSV);
+        SS_ADD_END(1);
+    }
 }
 
 void
@@ -571,23 +587,26 @@ void
 Perl_save_destructor(pTHX_ DESTRUCTORFUNC_NOCONTEXT_t f, void* p)
 {
     dVAR;
+    dSS_ADD;
 
     PERL_ARGS_ASSERT_SAVE_DESTRUCTOR;
 
-    SSCHECK(3);
-    SSPUSHDPTR(f);
-    SSPUSHPTR(p);
-    SSPUSHUV(SAVEt_DESTRUCTOR);
+    SS_ADD_DPTR(f);
+    SS_ADD_PTR(p);
+    SS_ADD_UV(SAVEt_DESTRUCTOR);
+    SS_ADD_END(3);
 }
 
 void
 Perl_save_destructor_x(pTHX_ DESTRUCTORFUNC_t f, void* p)
 {
     dVAR;
-    SSCHECK(3);
-    SSPUSHDXPTR(f);
-    SSPUSHPTR(p);
-    SSPUSHUV(SAVEt_DESTRUCTOR_X);
+    dSS_ADD;
+
+    SS_ADD_DXPTR(f);
+    SS_ADD_PTR(p);
+    SS_ADD_UV(SAVEt_DESTRUCTOR_X);
+    SS_ADD_END(3);
 }
 
 void
@@ -609,11 +628,12 @@ static void
 S_save_pushptri32ptr(pTHX_ void *const ptr1, const I32 i, void *const ptr2,
                        const int type)
 {
-    SSCHECK(4);
-    SSPUSHPTR(ptr1);
-    SSPUSHINT(i);
-    SSPUSHPTR(ptr2);
-    SSPUSHUV(type);
+    dSS_ADD;
+    SS_ADD_PTR(ptr1);
+    SS_ADD_INT(i);
+    SS_ADD_PTR(ptr2);
+    SS_ADD_UV(type);
+    SS_ADD_END(4);
 }
 
 void
@@ -652,11 +672,14 @@ Perl_save_helem_flags(pTHX_ HV *hv, SV *key, SV **sptr, const U32 flags)
     PERL_ARGS_ASSERT_SAVE_HELEM_FLAGS;
 
     SvGETMAGIC(*sptr);
-    SSCHECK(4);
-    SSPUSHPTR(SvREFCNT_inc_simple(hv));
-    SSPUSHPTR(newSVsv(key));
-    SSPUSHPTR(SvREFCNT_inc(*sptr));
-    SSPUSHUV(SAVEt_HELEM);
+    {
+        dSS_ADD;
+        SS_ADD_PTR(SvREFCNT_inc_simple(hv));
+        SS_ADD_PTR(newSVsv(key));
+        SS_ADD_PTR(SvREFCNT_inc(*sptr));
+        SS_ADD_UV(SAVEt_HELEM);
+        SS_ADD_END(4);
+    }
     save_scalar_at(sptr, flags);
     if (flags & SAVEf_KEEPOLDELEM)
        return;

diff --git a/scope.h b/scope.h
index f1d1929..5854850 100644 (file)
--- a/scope.h
+++ b/scope.h
@@ -74,8 +74,12 @@
 #define SCOPE_SAVES_SIGNAL_MASK 0
 #endif
 
-#define SSCHECK(need) if (PL_savestack_ix + (I32)(need) > PL_savestack_max) savestack_grow()
-#define SSGROW(need) if (PL_savestack_ix + (I32)(need) > PL_savestack_max) savestack_grow_cnt(need)
+/* the maximum number of entries that might be pushed using the SS_ADD*
+ * macros */
+#define SS_MAXPUSH 4
+
+#define SSCHECK(need) if (PL_savestack_ix + (I32)(need) + SS_MAXPUSH > PL_savestack_max) savestack_grow()
+#define SSGROW(need) if (PL_savestack_ix + (I32)(need) + SS_MAXPUSH > PL_savestack_max) savestack_grow_cnt(need + SS_MAXPUSH)
 #define SSPUSHINT(i) (PL_savestack[PL_savestack_ix++].any_i32 = (I32)(i))
 #define SSPUSHLONG(i) (PL_savestack[PL_savestack_ix++].any_long = (long)(i))
 #define SSPUSHBOOL(p) (PL_savestack[PL_savestack_ix++].any_bool = (p))
@@ -84,6 +88,40 @@
 #define SSPUSHPTR(p) (PL_savestack[PL_savestack_ix++].any_ptr = (void*)(p))
 #define SSPUSHDPTR(p) (PL_savestack[PL_savestack_ix++].any_dptr = (p))
 #define SSPUSHDXPTR(p) (PL_savestack[PL_savestack_ix++].any_dxptr = (p))
+
+/* SS_ADD*: newer, faster versions of the above. Don't mix the two sets of
+ * macros. These are fast because they save reduce accesses to the PL_
+ * vars and move the size check to the end. Doing the check last means
+ * that values in registers will have been pushed and no longer needed, so
+ * don't need saving around the call to grow. Also, tail-call elimination
+ * of the grow() can be done. These changes reduce the code of something
+ * like save_pushptrptr() to half its former size.
+ * Of course, doing the size check *after* pushing means we must always
+ * ensure there are SS_MAXPUSH free slots on the savestack
+ *
+ * These are for internal core use only and are subject to change */
+
+#define dSS_ADD \
+    I32 ix = PL_savestack_ix;     \
+    ANY *ssp = &PL_savestack[ix];
+
+#define SS_ADD_END(need) \
+    assert((need) <= SS_MAXPUSH);                               \
+    ix += (need);                                               \
+    PL_savestack_ix = ix;                                       \
+    assert(ix <= PL_savestack_max);                             \
+    if ((ix + SS_MAXPUSH) > PL_savestack_max) savestack_grow(); \
+    assert(PL_savestack_ix + SS_MAXPUSH <= PL_savestack_max);
+
+#define SS_ADD_INT(i)   ((ssp++)->any_i32 = (I32)(i))
+#define SS_ADD_LONG(i)  ((ssp++)->any_long = (long)(i))
+#define SS_ADD_BOOL(p)  ((ssp++)->any_bool = (p))
+#define SS_ADD_IV(i)    ((ssp++)->any_iv = (IV)(i))
+#define SS_ADD_UV(u)    ((ssp++)->any_uv = (UV)(u))
+#define SS_ADD_PTR(p)   ((ssp++)->any_ptr = (void*)(p))
+#define SS_ADD_DPTR(p)  ((ssp++)->any_dptr = (p))
+#define SS_ADD_DXPTR(p) ((ssp++)->any_dxptr = (p))
+
 #define SSPOPINT (PL_savestack[--PL_savestack_ix].any_i32)
 #define SSPOPLONG (PL_savestack[--PL_savestack_ix].any_long)
 #define SSPOPBOOL (PL_savestack[--PL_savestack_ix].any_bool)
@@ -93,6 +131,7 @@
 #define SSPOPDPTR (PL_savestack[--PL_savestack_ix].any_dptr)
 #define SSPOPDXPTR (PL_savestack[--PL_savestack_ix].any_dxptr)
 
+
 /*
 =head1 Callback Functions
 
@@ -202,10 +241,11 @@ scope has the given name. Name must be a literal string.
          save_destructor_x((DESTRUCTORFUNC_t)(f), (void*)(p))
 
 #define SAVESTACK_POS() \
-    STMT_START {                               \
-       SSCHECK(2);                             \
-       SSPUSHINT(PL_stack_sp - PL_stack_base); \
-       SSPUSHUV(SAVEt_STACK_POS);              \
+    STMT_START {                                  \
+        dSS_ADD;                                   \
+        SS_ADD_INT(PL_stack_sp - PL_stack_base);   \
+        SS_ADD_UV(SAVEt_STACK_POS);                \
+        SS_ADD_END(2);                             \
     } STMT_END
 
 #define SAVEOP()       save_op()
@@ -229,11 +269,12 @@ scope has the given name. Name must be a literal string.
 #define SAVECOMPILEWARNINGS() save_pushptr(PL_compiling.cop_warnings, SAVEt_COMPILE_WARNINGS)
 
 #define SAVESTACK_CXPOS() \
-    STMT_START {                                  \
-        SSCHECK(3);                               \
-        SSPUSHINT(cxstack[cxstack_ix].blk_oldsp); \
-        SSPUSHINT(cxstack_ix);                    \
-        SSPUSHUV(SAVEt_STACK_CXPOS);              \
+    STMT_START {                                   \
+        dSS_ADD;                                   \
+        SS_ADD_INT(cxstack[cxstack_ix].blk_oldsp); \
+        SS_ADD_INT(cxstack_ix);                    \
+        SS_ADD_UV(SAVEt_STACK_CXPOS);              \
+        SS_ADD_END(3);                             \
     } STMT_END
 
 #define SAVEPARSER(p) save_pushptr((p), SAVEt_PARSER)

diff --git a/sv.c b/sv.c
index 35d295e..397d992 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -3795,13 +3795,14 @@ S_glob_assign_ref(pTHX_ SV *const dstr, SV *const sstr)
               gain a name somehow before leave_scope. */
            if (stype == SVt_PVCV) {
                /* There is no save_pushptrptrptr.  Creating it for this
-                  one call site would be overkill.  So inline the ss push
+                  one call site would be overkill.  So inline the ss add
                   routines here. */
-               SSCHECK(4);
-               SSPUSHPTR(dstr);
-               SSPUSHPTR(location);
-               SSPUSHPTR(SvREFCNT_inc(*location));
-               SSPUSHUV(SAVEt_GVSLOT);
+                dSS_ADD;
+               SS_ADD_PTR(dstr);
+               SS_ADD_PTR(location);
+               SS_ADD_PTR(SvREFCNT_inc(*location));
+               SS_ADD_UV(SAVEt_GVSLOT);
+               SS_ADD_END(4);
            }
            else SAVEGENERICSV(*location);
        }