From 9e99fbb3940bfab6ce2638107363e928bcca917a Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Wed, 21 Sep 2022 20:46:18 +0000 Subject: [PATCH] AELEMFASTLEX_STORE - support negative keys, skip unnecessary check This commit: * Adds support for negative keys, as per the original AELEMFAST_LEX * Changes an if() check for a "useless assignment to a temporary" into an assert, since this condition should never be true when the LHS is the result of an array fetch. --- peep.c | 3 +-- pp_hot.c | 18 +++++++++--------- t/perf/opcount.t | 8 ++++---- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/peep.c b/peep.c index c18a305..1319745 100644 --- a/peep.c +++ b/peep.c @@ -3958,8 +3958,7 @@ Perl_rpeep(pTHX_ OP *o) if (!(o->op_private & (OPpASSIGN_BACKWARDS|OPpASSIGN_CV_TO_GV)) && (lval->op_type == OP_NULL) && (lval->op_private == 2) && - (cBINOPx(lval)->op_first->op_type == OP_AELEMFAST_LEX) && - ((I8)(cBINOPx(lval)->op_first->op_private) >= 0) + (cBINOPx(lval)->op_first->op_type == OP_AELEMFAST_LEX) ) { OP * lex = cBINOPx(lval)->op_first; /* SASSIGN's bitfield flags, such as op_moresib and diff --git a/pp_hot.c b/pp_hot.c index e64dbd0..30a5b35 100644 --- a/pp_hot.c +++ b/pp_hot.c @@ -185,10 +185,9 @@ PP(pp_aelemfastlex_store) /* Inlined, simplified pp_aelemfast here */ assert(SvTYPE(av) == SVt_PVAV); - assert(key >= 0); /* inlined av_fetch() for simple cases ... */ - if (!SvRMAGICAL(av) && key <= AvFILLp(av)) { + if (!SvRMAGICAL(av) && key >=0 && key <= AvFILLp(av)) { targ = AvARRAY(av)[key]; } /* ... else do it the hard way */ @@ -206,13 +205,14 @@ PP(pp_aelemfastlex_store) if (UNLIKELY(TAINT_get) && !SvTAINTED(val)) TAINT_NOT; - if ( - UNLIKELY(SvTEMP(targ)) && !SvSMAGICAL(targ) && SvREFCNT(targ) == 1 && - (!isGV_with_GP(targ) || SvFAKE(targ)) && ckWARN(WARN_MISC) - ) - Perl_warner(aTHX_ - packWARN(WARN_MISC), "Useless assignment to a temporary" - ); + /* This assertion is a deviation from pp_sassign, which uses an if() + * condition to check for "Useless assignment to a temporary" and + * warns if the condition is true. Here, the condition should NEVER + * be true when the LHS is the result of an array fetch. The + * assertion is here as a final check that this remains the case. + */ + assert(!(SvTEMP(targ) && SvREFCNT(targ) == 1 && !SvSMAGICAL(targ))); + SvSetMagicSV(targ, val); SETs(targ); diff --git a/t/perf/opcount.t b/t/perf/opcount.t index ffb8782..cca8dec 100644 --- a/t/perf/opcount.t +++ b/t/perf/opcount.t @@ -898,13 +898,13 @@ test_opcount(0, "simple aelemfast_lex + sassign replacement", # aelemfast_lex + sassign are not replaced by a combined OP # when key <0 (not handled, to keep the pp_ function simple -test_opcount(0, "no aelemfast_lex + sassign replacement with neg key", +test_opcount(0, "aelemfast_lex + sassign replacement with neg key", sub { my @x = (1,2); $x[-1] = 7 }, { - aelemfast_lex => 1, - aelemfastlex_store => 0, + aelemfast_lex => 0, + aelemfastlex_store => 1, padav => 1, - sassign => 1, + sassign => 0, }); # aelemfast_lex + sassign optimization does not disrupt multideref -- 1.8.3.1