From 135950349c273d11a1855b7b6bce4a42df3c0954 Mon Sep 17 00:00:00 2001 From: Karl Williamson Date: Wed, 13 Aug 2025 11:53:29 -0600 Subject: [PATCH 1/2] Move prototype definition of SvPV_helper to embed.fnc It's usually a bad idea to try to work around a limitation in common code by copy-pasting and then modifiying to taste. Fixes/improvements to the common code rarely get propagated to the outlier. I wrote code in 1ef9039bccb that did just this for the prototype definition of SvPV_helper, because the place where it really belongs, embed.fnc, couldn't (and still doesn't) handle function pointers as arguments (patches welcome). I should have at least added a comment to the common code noting the existence of this outlier. It turns out that that limitation can be worked around by declaring a typedef of the pointer, and then using that in embed.fnc. That's what this commit does. This commit removes the final instance of duplicating the work of embed.fnc in the core, except for some in the regex system whose comments say the reason is to avoid making a typedef public. I haven't investigated these further. --- embed.fnc | 7 +++++++ embed.h | 1 + proto.h | 5 +++++ sv.h | 12 ++---------- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/embed.fnc b/embed.fnc index aceb5014e14b..14f27dbace25 100644 --- a/embed.fnc +++ b/embed.fnc @@ -3358,6 +3358,13 @@ CRdmp |char * |sv_2pvbyte_nolen \ Adp |char * |sv_2pv_flags |NN SV * const sv \ |NULLOK STRLEN * const lp \ |const U32 flags +Cip |char * |SvPV_helper |NN SV * const sv \ + |NN STRLEN * const lp \ + |const U32 flags \ + |const PL_SvPVtype type \ + |NN Perl_SvPV_helper_non_trivial_t non_trivial \ + |const bool or_null \ + |const U32 return_flags Cdmp |char * |sv_pvn_force |NN SV *sv \ |NULLOK STRLEN *lp Adp |char * |sv_pvn_force_flags \ diff --git a/embed.h b/embed.h index ae046a6173e8..9dd285c36018 100644 --- a/embed.h +++ b/embed.h @@ -109,6 +109,7 @@ # define SvNV(a) Perl_SvNV(aTHX_ a) # define SvNV_nomg(a) Perl_SvNV_nomg(aTHX_ a) # define SvPVXtrue(a) Perl_SvPVXtrue(aTHX_ a) +# define SvPV_helper(a,b,c,d,e,f,g) Perl_SvPV_helper(aTHX_ a,b,c,d,e,f,g) # define SvREFCNT_dec_ret_NULL(a) Perl_SvREFCNT_dec_ret_NULL(aTHX_ a) # define SvTRUE(a) Perl_SvTRUE(aTHX_ a) # define SvTRUE_NN(a) Perl_SvTRUE_NN(aTHX_ a) diff --git a/proto.h b/proto.h index 138d33b2b112..7b6df411eb2e 100644 --- a/proto.h +++ b/proto.h @@ -9680,6 +9680,11 @@ Perl_SvPVXtrue(pTHX_ SV *sv) # define PERL_ARGS_ASSERT_SVPVXTRUE \ assert(sv) +PERL_STATIC_INLINE char * +Perl_SvPV_helper(pTHX_ SV * const sv, STRLEN * const lp, const U32 flags, const PL_SvPVtype type, Perl_SvPV_helper_non_trivial_t non_trivial, const bool or_null, const U32 return_flags); +# define PERL_ARGS_ASSERT_SVPV_HELPER \ + assert(sv); assert(lp); assert(non_trivial) + PERL_STATIC_INLINE void Perl_SvREFCNT_dec(pTHX_ SV *sv); # define PERL_ARGS_ASSERT_SVREFCNT_DEC diff --git a/sv.h b/sv.h index 181aedfb3173..708aa7dc0131 100644 --- a/sv.h +++ b/sv.h @@ -1958,16 +1958,8 @@ typedef enum { SvPVbyte_pure_type_ } PL_SvPVtype; -START_EXTERN_C - -/* When this code was written, embed.fnc could not handle function pointer - * parameters; perhaps it still can't */ -#ifndef PERL_NO_INLINE_FUNCTIONS -PERL_STATIC_INLINE char* -Perl_SvPV_helper(pTHX_ SV *const sv, STRLEN *const lp, const U32 flags, const PL_SvPVtype type, char * (*non_trivial)(pTHX_ SV *, STRLEN * const, const U32), const bool or_null, const U32 return_flags); -#endif - -END_EXTERN_C +typedef char * (*Perl_SvPV_helper_non_trivial_t)(pTHX_ SV *, STRLEN * const, + const U32); /* This test is "is there a cached PV that we can use directly?" * We can if From f10b118e281e7ec2c0275e0df747de2f2da07aa6 Mon Sep 17 00:00:00 2001 From: Karl Williamson Date: Wed, 13 Aug 2025 12:12:56 -0600 Subject: [PATCH 2/2] Force SvPV_helper to be always inline (when the compiler cooperates, that is) I compiled toke.s using -O0 and looked at the output. It had a full copy of SvPV_helper in it. But this function is called with various constants that make much of the code dead when constant folded. That was not being done here. This commit changes to force inlining. Looking at a revised toke.s, I saw that SvPV_helper was inserted multiple times in the code, and that each occurrence had significantly fewer instructions than the original. I infer that the compiler did the constant folding and pruned out the dead code. --- embed.fnc | 2 +- proto.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/embed.fnc b/embed.fnc index 14f27dbace25..7c66585269be 100644 --- a/embed.fnc +++ b/embed.fnc @@ -3358,7 +3358,7 @@ CRdmp |char * |sv_2pvbyte_nolen \ Adp |char * |sv_2pv_flags |NN SV * const sv \ |NULLOK STRLEN * const lp \ |const U32 flags -Cip |char * |SvPV_helper |NN SV * const sv \ +CIp |char * |SvPV_helper |NN SV * const sv \ |NN STRLEN * const lp \ |const U32 flags \ |const PL_SvPVtype type \ diff --git a/proto.h b/proto.h index 7b6df411eb2e..4bf2ee0ea1df 100644 --- a/proto.h +++ b/proto.h @@ -9680,8 +9680,9 @@ Perl_SvPVXtrue(pTHX_ SV *sv) # define PERL_ARGS_ASSERT_SVPVXTRUE \ assert(sv) -PERL_STATIC_INLINE char * -Perl_SvPV_helper(pTHX_ SV * const sv, STRLEN * const lp, const U32 flags, const PL_SvPVtype type, Perl_SvPV_helper_non_trivial_t non_trivial, const bool or_null, const U32 return_flags); +PERL_STATIC_FORCE_INLINE char * +Perl_SvPV_helper(pTHX_ SV * const sv, STRLEN * const lp, const U32 flags, const PL_SvPVtype type, Perl_SvPV_helper_non_trivial_t non_trivial, const bool or_null, const U32 return_flags) + __attribute__always_inline__; # define PERL_ARGS_ASSERT_SVPV_HELPER \ assert(sv); assert(lp); assert(non_trivial)