refactor gv_add_by_type

gv_add_by_type was added in commit d5713896ec in 5.11.0 . Improve
gv_add_by_type by making it return the newly created SV*, instead of the
the GV *, which the caller must deref both the GV head to get svu and
then deref a slice into the GP, even though it already derefed svu and GP
right before, to figure out whether to call gv_add_by_type in the first
place. The original version of this patch had gv_add_by_type returning a
SV ** to ensure lvalue-ness but it was discovered it wasn't needed and not
smart.

-rename gv_add_by_type since it was removed from public api and its proto
 changed
-remove null check since it is impossible to pass null through GvAVn(),
 and unlikely with gv_AVadd, null segvs reliably crash in the rare case of
 a problem
-instead of S_gv_init_svtype and gv_add_by_type using a tree of logic/
 conditional jumps in asm, use a lookup table, GPe (e=enum or entry)
 enums are identical to offsets into the GP struct, all of then fit under
 0xFF, if the CC and CPU arch wants, CC can load the const once into a
 register, then use the number for the 2nd deref, then use the number again
 as an arg to gv_add_by_type, the low (&~0xf) or high (<<2) 2 bits in a
 GPe can be used for something else in the future since GPe is pointer
 aligned
-SVt_LAST triggers "panic: sv_upgrade to unknown type", so use that value
 for entries of a GP which are not SV head *s and are invalid to pass as
 an arg
-remove the tree of logic in S_gv_init_svtype, replace with a table
-S_gv_init_svtype is now tail call friendly and very small
-change the GV**n to be rvalues only, assigning to GV**n is probably a
 memory leak
-fix 1 core GV**n as lvalue use
-GvSVn's unusual former definition is from commit 547f15c3f9 in 2005
 and DEFSV as lvalue is gone in core as of commit 414bf5ae08 from 2008
 since all the GV**n macros are now rvalues, this goes too
-PTRPTR2IDX and PTRSIZELOG2 could use better names
-in pp_rv2av dont declare strings like that VC linker won't dedup that, and
 other parts of core also have "an ARRAY", perl521.dll previously had 2
 "an ARRAY" and "a HASH" strings in it due to this

before VC 2003 32 perl521.dll .text 0xc8813 in machine code bytes after
.text 0xc8623

1 files changed, 81 insertions, 50 deletions

diff --git a/gv.c b/gv.c
index 68018162f1..5a05afa825 100644
--- a/gv.c
+++ b/gv.c
@@ -41,53 +41,56 @@ Perl stores its global variables.
 static const char S_autoload[] = "AUTOLOAD";
 static const STRLEN S_autolen = sizeof(S_autoload)-1;
 
-GV *
-Perl_gv_add_by_type(pTHX_ GV *gv, svtype type)
+SV *
+Perl_gv_add_by_type_p(pTHX_ GV *gv, gv_add_type type)
 {
     SV **where;
+    SV * sv;
+    PERL_ARGS_ASSERT_GV_ADD_BY_TYPE_P;
 
-    if (
-        !gv
-     || (
-            SvTYPE((const SV *)gv) != SVt_PVGV
+    if ( SvTYPE((const SV *)gv) != SVt_PVGV
          && SvTYPE((const SV *)gv) != SVt_PVLV
-        )
     ) {
 	const char *what;
-	if (type == SVt_PVIO) {
+	if (type == GPe_IO) {
 	    /*
 	     * if it walks like a dirhandle, then let's assume that
 	     * this is a dirhandle.
 	     */
 	    what = OP_IS_DIRHOP(PL_op->op_type) ?
 		"dirhandle" : "filehandle";
-	} else if (type == SVt_PVHV) {
+	} else if (type == GPe_HV) {
 	    what = "hash";
 	} else {
-	    what = type == SVt_PVAV ? "array" : "scalar";
+	    what = type == GPe_AV ? "array" : "scalar";
 	}
 	/* diag_listed_as: Bad symbol for filehandle */
 	Perl_croak(aTHX_ "Bad symbol for %s", what);
     }
 
-    if (type == SVt_PVHV) {
-	where = (SV **)&GvHV(gv);
-    } else if (type == SVt_PVAV) {
-	where = (SV **)&GvAV(gv);
-    } else if (type == SVt_PVIO) {
-	where = (SV **)&GvIOp(gv);
-    } else {
-	where = &GvSV(gv);
-    }
+    where = (SV **)((Size_t)GvGP(gv)+ (Size_t)type);
 
-    if (!*where)
-    {
-	*where = newSV_type(type);
-	    if (type == SVt_PVAV && GvNAMELEN(gv) == 3
-	     && strnEQ(GvNAME(gv), "ISA", 3))
-	    sv_magic(*where, (SV *)gv, PERL_MAGIC_isa, NULL, 0);
+    sv = *where;
+    if (!sv) {
+/* this is table of GP members to their SV types, SVt_LAST triggers a panic */
+	static const U8 addtype_to_svtype
+#if PTRSIZE == 8
+             /*gp_sv   , gp_io   , gp_cv   , cvgn/cnt, gp_hv   , gp_av */
+        [6] = {SVt_NULL, SVt_PVIO, SVt_LAST, SVt_LAST, SVt_PVHV, SVt_PVAV};
+#elif PTRSIZE == 4
+             /*gp_sv   , gp_io   , gp_cv   , gp_cvgen, gp_rfcnt, gp_hv   , gp_av */
+        [7] = {SVt_NULL, SVt_PVIO, SVt_LAST, SVt_LAST, SVt_LAST, SVt_PVHV, SVt_PVAV};
+#else
+#  error unknown pointer size
+#endif
+	svtype svtypevar = (svtype)addtype_to_svtype[PTRPTR2IDX(type)];
+
+	assert(PTRPTR2IDX(type) < sizeof(addtype_to_svtype));
+	sv = *where = newSV_type(svtypevar);
+	if (type == GPe_AV && memEQs(GvNAME(gv), GvNAMELEN(gv), "ISA"))
+	    sv_magic(sv, (SV *)gv, PERL_MAGIC_isa, NULL, 0);
     }
-    return gv;
+    return sv;
 }
 
 GV *
@@ -459,32 +462,60 @@ Perl_gv_init_pvn(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len, U32 flag
 STATIC void
 S_gv_init_svtype(pTHX_ GV *gv, const svtype sv_type)
 {
-    PERL_ARGS_ASSERT_GV_INIT_SVTYPE;
-
-    switch (sv_type) {
-    case SVt_PVIO:
-	(void)GvIOn(gv);
-	break;
-    case SVt_PVAV:
-	(void)GvAVn(gv);
-	break;
-    case SVt_PVHV:
-	(void)GvHVn(gv);
-	break;
+    Size_t addtype;
+#define SGVINIT_SKIP 0xFF
 #ifdef PERL_DONT_CREATE_GVSV
-    case SVt_NULL:
-    case SVt_PVCV:
-    case SVt_PVFM:
-    case SVt_PVGV:
-	break;
-    default:
-	if(GvSVn(gv)) {
-	    /* Work round what appears to be a bug in Sun C++ 5.8 2005/10/13
-	       If we just cast GvSVn(gv) to void, it ignores evaluating it for
-	       its side effect */
-	}
+#  define SGVINIT_SV GPe_SV
+#else
+#  define SGVINIT_SV SGVINIT_SKIP
 #endif
-    }
+    static const U8 svtype2add [] = {
+	/*SVt_NULL,	0 */
+        SGVINIT_SKIP,
+	/*SVt_IV,	1 */
+        SGVINIT_SV,
+	/*SVt_NV,	2 */
+        SGVINIT_SV,
+	/*SVt_PV,	3 */
+        SGVINIT_SV,
+	/*SVt_INVLIST,	4 implemented as a PV */
+        SGVINIT_SV,
+	/*SVt_PVIV,	5 */
+        SGVINIT_SV,
+	/*SVt_PVNV,	6 */
+        SGVINIT_SV,
+	/*SVt_PVMG,	7 */
+        SGVINIT_SV,
+	/*SVt_REGEXP,	8 */
+        SGVINIT_SV,
+	/*SVt_PVGV,	9 */
+        SGVINIT_SKIP,
+	/*SVt_PVLV,	10 */
+        SGVINIT_SV,
+	/*SVt_PVAV,	11 */
+        GPe_AV,
+	/*SVt_PVHV,	12 */
+        GPe_HV,
+	/*SVt_PVCV,	13 */
+        SGVINIT_SKIP,
+	/*SVt_PVFM,	14 */
+        SGVINIT_SKIP,
+	/*SVt_PVIO,	15 */
+        GPe_IO,
+	/*SVt_LAST	keep last in enum. used to size arrays */
+        /* invalid, this is slot 0x10, dont define it so this array is
+        a nice 16 bytes long */
+    };
+    PERL_ARGS_ASSERT_GV_INIT_SVTYPE;
+    addtype = svtype2add[sv_type];
+    if(addtype != SGVINIT_SKIP) {
+        SV ** where = (SV **)((Size_t)GvGP(gv)+ addtype);
+        if (!*where)
+            gv_add_by_type_p(gv, (gv_add_type)addtype);
+    }
+    return;
+#undef SGVINIT_SV
+#undef SGVINIT_SKIP
 }
 
 static void core_xsub(pTHX_ CV* cv);