The layout for struct block_loop under ithreads can be simplified.
Nicholas Clark [Sat, 26 Jan 2008 21:55:51 +0000 (21:55 +0000)]
Instead of wedging the pad offset into a void* iterdata, and always
storing PL_comppad even when it isn't used, instead do this:

PAD        *oldcomppad; /* Also used for the GV, if targoffset is 0 */
/* This is also accesible via cx->blk_loop.my_op->op_targ */
PADOFFSET  targoffset;

and store the GV pointer in oldcompad. Pointers to pointers seems
cleaner. This also allows us to eliminate the flag bit CXp_PADVAR.

p4raw-id: //depot/perl@33081

cop.h
pp_ctl.c
sv.c

diff --git a/cop.h b/cop.h
index 4af6b8f..b79cc19 100644 (file)
--- a/cop.h
+++ b/cop.h
@@ -436,8 +436,9 @@ struct block_loop {
     /* (except for non_ithreads we need to modify next_op in pp_ctl.c, hence
        why next_op is conditionally defined below.)  */
 #ifdef USE_ITHREADS
-    void *     iterdata;
-    PAD                *oldcomppad;
+    PAD                *oldcomppad; /* Also used for the GV, if targoffset is 0 */
+    /* This is also accesible via cx->blk_loop.my_op->op_targ */
+    PADOFFSET  targoffset;
 #else
     OP *       next_op;
     SV **      itervar;
@@ -460,18 +461,19 @@ struct block_loop {
 
 #ifdef USE_ITHREADS
 #  define CxITERVAR(c)                                                 \
-       ((c)->blk_loop.iterdata                                         \
-        ? (CxPADLOOP(cx)                                               \
-           ? &CX_CURPAD_SV( (c)->blk_loop,                             \
-                   INT2PTR(PADOFFSET, (c)->blk_loop.iterdata))         \
-           : &GvSV((GV*)(c)->blk_loop.iterdata))                       \
+       ((c)->blk_loop.oldcomppad                                       \
+        ? (CxPADLOOP(c)                                                \
+           ? &CX_CURPAD_SV( (c)->blk_loop, (c)->blk_loop.targoffset )  \
+           : &GvSV((GV*)(c)->blk_loop.oldcomppad))                     \
         : (SV**)NULL)
-#  define CX_ITERDATA_SET(cx,idata)                                    \
-       CX_CURPAD_SAVE(cx->blk_loop);                                   \
-       cx->blk_loop.iterdata = (idata);
+#  define CX_ITERDATA_SET(cx,idata,o)                                  \
+       if ((cx->blk_loop.targoffset = (o)))                            \
+           CX_CURPAD_SAVE(cx->blk_loop);                               \
+       else                                                            \
+           cx->blk_loop.oldcomppad = (idata);
 #else
 #  define CxITERVAR(c)         ((c)->blk_loop.itervar)
-#  define CX_ITERDATA_SET(cx,ivar)                                     \
+#  define CX_ITERDATA_SET(cx,ivar,o)                                   \
        cx->blk_loop.itervar = (SV**)(ivar);
 #endif
 #define CxLABEL(c)     (0 + (c)->blk_oldcop->cop_label)
@@ -492,15 +494,15 @@ struct block_loop {
        PUSHLOOP_OP_NEXT;                                               \
        cx->blk_loop.state_u.ary.ary = NULL;                            \
        cx->blk_loop.state_u.ary.ix = 0;                                \
-       CX_ITERDATA_SET(cx,NULL);
+       CX_ITERDATA_SET(cx, NULL, 0);
 
-#define PUSHLOOP_FOR(cx, dat, s)                                       \
+#define PUSHLOOP_FOR(cx, dat, s, offset)                               \
        cx->blk_loop.resetsp = s - PL_stack_base;                       \
        cx->blk_loop.my_op = cLOOP;                                     \
        PUSHLOOP_OP_NEXT;                                               \
        cx->blk_loop.state_u.ary.ary = NULL;                            \
        cx->blk_loop.state_u.ary.ix = 0;                                \
-       CX_ITERDATA_SET(cx,dat);
+       CX_ITERDATA_SET(cx, dat, offset);
 
 #define POPLOOP(cx)                                                    \
        if (CxTYPE(cx) == CXt_LOOP_LAZYSV) {                            \
@@ -680,10 +682,9 @@ struct context {
 /* private flags for CXt_LOOP */
 #define CXp_FOR_DEF    0x10    /* foreach using $_ */
 #ifdef USE_ITHREADS
-#  define CXp_PADVAR   0x20    /* itervar lives on pad, iterdata has pad
-                                  offset; if not set, iterdata holds GV* */
-#  define CxPADLOOP(c) (CxTYPE_is_LOOP(c) && ((c)->cx_type & (CXp_PADVAR)))
+#  define CxPADLOOP(c) ((c)->blk_loop.targoffset)
 #endif
+
 /* private flags for CXt_SUBST */
 #define CXp_ONCE       0x10    /* What was sbu_once in struct subst */
 
index 9c6e2b1..87e2d40 100644 (file)
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -1837,7 +1837,7 @@ PP(pp_enteriter)
     SV **svp;
     U8 cxtype = CXt_LOOP_FOR;
 #ifdef USE_ITHREADS
-    void *iterdata;
+    PAD *iterdata;
 #endif
 
     ENTER;
@@ -1853,8 +1853,7 @@ PP(pp_enteriter)
 #ifndef USE_ITHREADS
        svp = &PAD_SVl(PL_op->op_targ);         /* "my" variable */
 #else
-       iterdata = INT2PTR(void*, PL_op->op_targ);
-       cxtype |= CXp_PADVAR;
+       iterdata = NULL;
 #endif
     }
     else {
@@ -1863,7 +1862,7 @@ PP(pp_enteriter)
        SAVEGENERICSV(*svp);
        *svp = newSV(0);
 #ifdef USE_ITHREADS
-       iterdata = (void*)gv;
+       iterdata = (PAD*)gv;
 #endif
     }
 
@@ -1874,9 +1873,9 @@ PP(pp_enteriter)
 
     PUSHBLOCK(cx, cxtype, SP);
 #ifdef USE_ITHREADS
-    PUSHLOOP_FOR(cx, iterdata, MARK);
+    PUSHLOOP_FOR(cx, iterdata, MARK, PL_op->op_targ);
 #else
-    PUSHLOOP_FOR(cx, svp, MARK);
+    PUSHLOOP_FOR(cx, svp, MARK, /*Not used*/);
 #endif
     if (PL_op->op_flags & OPf_STACKED) {
        SV *maybe_ary = POPs;
diff --git a/sv.c b/sv.c
index 6d60eeb..d30f077 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -10557,13 +10557,14 @@ Perl_cx_dup(pTHX_ PERL_CONTEXT *cxs, I32 ix, I32 max, CLONE_PARAMS* param)
                    = av_dup_inc(ncx->blk_loop.state_u.ary.ary, param);
            case CXt_LOOP_LAZYIV:
            case CXt_LOOP_PLAIN:
-               ncx->blk_loop.iterdata  = (CxPADLOOP(ncx)
-                                          ? ncx->blk_loop.iterdata
-                                          : gv_dup((GV*)ncx->blk_loop.iterdata,
-                                                   param));
-               ncx->blk_loop.oldcomppad
-                   = (PAD*)ptr_table_fetch(PL_ptr_table,
-                                           ncx->blk_loop.oldcomppad);
+               if (CxPADLOOP(ncx)) {
+                   ncx->blk_loop.oldcomppad
+                       = (PAD*)ptr_table_fetch(PL_ptr_table,
+                                               ncx->blk_loop.oldcomppad);
+               } else {
+                   ncx->blk_loop.oldcomppad
+                       = (PAD*)gv_dup((GV*)ncx->blk_loop.oldcomppad, param);
+               }
                break;
            case CXt_FORMAT:
                ncx->blk_format.cv      = cv_dup(ncx->blk_format.cv, param);