Skip to content

Commit fa7a1e4

Browse files
bulk88iabyn
authored andcommitted
make sv_backoff tailcall friendly
Reorder the body of Perl_sv_backoff slightly to make it more tail-call friendly, and change its signature from returning an int (always 0) to void. sv_backoff has only 1.5 function calls in it, there is a memcpy of a U32 * for alignment reasons (I wont discuss U32_ALIGNMENT_REQUIRED) inside of SvOOK_offset, and the explicit Move()/memmove. GCC and clang often inline memcpy/memmove when the length is a constant and is small. Sometimes a CC might also do unaligned memory reads if OS/CPU allows it http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20130513/174807.html so I'll assume memcpy by short constant isn't a func call for discussion. By moving SvFLAGS modification before the one and only func call, and changing the return type to void, there is no code to execute after the Move func call so the CC, if it wants (OS/ABI/CPU, specifically I am thinking about x86-64) can tailcall jump to memmove. Also var sv can be stored in a cheaper vol reg since it is not saved around any func calls (SvFLAGS set was moved) assuming the memcpy by short constant was inlined. The before machine code size of Perl_sv_backoff with VC 2003 -O1 was 0x6d bytes. After size is 0x61. .text section size of perl523.dll was after was 0xD2733 bytes long, before was 0xD2743 bytes long. VC perl does not inline memcpys by default. In commit a0d0e21 "perl 5.000" the return 0 was added. The int ret type is from day 1 of sv_backoff function existing/day 1 of SV *s from commit 7907280 "perl 5.0 alpha 2". str_backoff didn't exist AFAIK, only str_grow would retake the memory at the start of the block. Since sv_backoff is usually used in a "&& func()" macro (SvOOK_off), it needed a non void ret type, a simple ", 0" in the macro fixes that. All CCs optimize and remove "if(0)" machine instructions so the ", 0" is optimized away in the perl binary.
1 parent 6fc2106 commit fa7a1e4

File tree

5 files changed

+17
-7
lines changed

5 files changed

+17
-7
lines changed

embed.fnc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1374,7 +1374,7 @@ Apd |I32 |sv_true |NULLOK SV *const sv
13741374
sd |void |sv_add_arena |NN char *const ptr|const U32 size \
13751375
|const U32 flags
13761376
#endif
1377-
Apdn |int |sv_backoff |NN SV *const sv
1377+
Apdn |void |sv_backoff |NN SV *const sv
13781378
Apd |SV* |sv_bless |NN SV *const sv|NN HV *const stash
13791379
#if defined(PERL_DEBUG_READONLY_COW)
13801380
p |void |sv_buf_to_ro |NN SV *sv

pod/perldelta.pod

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,13 @@ well.
329329

330330
=item *
331331

332-
XXX
332+
L<perlapi/sv_backoff> had its return type changed fron C<int> to C<void>. It
333+
previously has always returned C<0> since 5.000 stable but that was
334+
undocumented. Although C<sv_backoff> is marked as public API, XS code is not
335+
expected to be impacted since the proper API call would be through public API
336+
C<sv_setsv(sv, &PL_sv_undef)>, or quasi-public C<SvOOK_off>, or non-public
337+
C<SvOK_off> calls, and the return value of C<sv_backoff> was previously a
338+
meaningless constant that can be rewritten as C<(sv_backoff(sv),0)>.
333339

334340
=back
335341

proto.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2897,7 +2897,7 @@ PERL_CALLCONV char* Perl_sv_2pvutf8(pTHX_ SV *sv, STRLEN *const lp);
28972897
PERL_CALLCONV UV Perl_sv_2uv_flags(pTHX_ SV *const sv, const I32 flags);
28982898
#define PERL_ARGS_ASSERT_SV_2UV_FLAGS \
28992899
assert(sv)
2900-
PERL_CALLCONV int Perl_sv_backoff(SV *const sv);
2900+
PERL_CALLCONV void Perl_sv_backoff(SV *const sv);
29012901
#define PERL_ARGS_ASSERT_SV_BACKOFF \
29022902
assert(sv)
29032903
PERL_CALLCONV SV* Perl_sv_bless(pTHX_ SV *const sv, HV *const stash);

sv.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,7 +1525,11 @@ wrapper instead.
15251525
=cut
15261526
*/
15271527

1528-
int
1528+
/* prior to 5.000 stable, this function returned the new OOK-less SvFLAGS
1529+
prior to 5.23.4 this function always returned 0
1530+
*/
1531+
1532+
void
15291533
Perl_sv_backoff(SV *const sv)
15301534
{
15311535
STRLEN delta;
@@ -1541,9 +1545,9 @@ Perl_sv_backoff(SV *const sv)
15411545

15421546
SvLEN_set(sv, SvLEN(sv) + delta);
15431547
SvPV_set(sv, SvPVX(sv) - delta);
1544-
Move(s, SvPVX(sv), SvCUR(sv)+1, char);
15451548
SvFLAGS(sv) &= ~SVf_OOK;
1546-
return 0;
1549+
Move(s, SvPVX(sv), SvCUR(sv)+1, char);
1550+
return;
15471551
}
15481552

15491553
/*

sv.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -949,7 +949,7 @@ in gv.h: */
949949

950950
#define SvOOK(sv) (SvFLAGS(sv) & SVf_OOK)
951951
#define SvOOK_on(sv) (SvFLAGS(sv) |= SVf_OOK)
952-
#define SvOOK_off(sv) ((void)(SvOOK(sv) && sv_backoff(sv)))
952+
#define SvOOK_off(sv) ((void)(SvOOK(sv) && (sv_backoff(sv),0)))
953953

954954
#define SvFAKE(sv) (SvFLAGS(sv) & SVf_FAKE)
955955
#define SvFAKE_on(sv) (SvFLAGS(sv) |= SVf_FAKE)

0 commit comments

Comments
 (0)