-
Notifications
You must be signed in to change notification settings - Fork 602
Optimize some use of SvUOK() to SvIsUV() #23914
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: blead
Are you sure you want to change the base?
Conversation
This would make it easier to receive the result with `bool` type. Note that (implicit) casting to C99 `_Bool` automatically converts any nonzero scalar values to 1, so that this change is technically not necessary in C99 which perl now requires to compile. But I think it will keep the code less surprising.
SvUOK(x) inside a block guarded by SvIV_please_nomg(x) can be replaced by SvIsUV(x) because SvIV_please_nomg implies SvIOK. This will save a few code size and runtime CPU cycles, because many CPUs can do single-bit tests like SvIsUV in fewer instructions than multi-bit tests like SvUOK.
Because SvUOK is essentially (SvIOK && SvIsUV), SvUOK is redundant in the block where SvIOK is guaranteed to hold true.
| (SvPVX_const(sv) == PL_No)) | ||
|
|
||
| #define SvIsUV(sv) (SvFLAGS(sv) & SVf_IVisUV) | ||
| #define SvIsUV(sv) ((SvFLAGS(sv) & SVf_IVisUV) != 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change doesn't seem necessary - surely the truthiness is already being returned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this change is technically not necessary in C99, but I think this will help keeping the code less surprising.
SvFLAGS(sv) & SVf_IVisUV takes 0 or 0x80000000 and assigning this result into bool value (which is not guaranteed to be as wide as 0x80000000) is not intuitive I think (although this is not real problem in C99).
|
@t-a-k - did you observe a decrease in code size or runtime performance improvement? (I haven't built perl from this PR and compared it to blead yet.) A quick play with godbolt.org suggests that a perl built with gcc < 15 or clang < 10 might not see any instruction or runtime throughput improvements. On recent versions of gcc (15.1) & clang (21.1.0), a simple function analogous to whereas a so there is theoretically a single instruction saving there. Putting the and and three different instructions for the i.e. The compilers (both using gcc 14.2 produces the same instructions as before for the simple |
Is this really In my observation on Debian 13 with gcc 14.2.0, bool auvok = SvUOK(svl);
...
if (auvok) ...compiles to something like (sorry for AT&T syntax) and $0x80000100,%eax // SVf_IVisUV | SVf_IOK
cmp $0x80000100,%eax
je ...or not %r8d
and $0x80000100,%r8d
je ...whereas a test %ecx,%ecx
js ...so the conclusion remains the same, there is theoretically a single instruction saving. In addition, With this change, overall decrease in perl code size is 16 bytes on my build: More specifically, |
I do apologise, the brackets around the flags in my
So the pp.c & pp_hot.c changes LGTM. |
I found that some use of SvUOK() in conditional blocks like
if (SvIOK(sv)) { ... }orif (SvIV_please_nomg(sv)) { ... }are redundant and can be safely replaced with SvIsUV, asSvUOK(sv)is essentiallySvIOK(sv) && SvIsUV(sv)andSvIOK(sv)is already asserted by the outer conditional.This change will save a few code size and runtime CPU cycles, because many CPUs can do single-bit tests like SvIsUV in fewer instructions than multi-bit tests like SvUOK.