Skip to content

Use compiler builtins to detect "simple common cases" in pp_add, pp_subtract, and pp_multiply #23503

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

Open
wants to merge 2 commits into
base: blead
Choose a base branch
from

Conversation

t-a-k
Copy link
Contributor

@t-a-k t-a-k commented Jul 29, 2025

This will hopefully make the code faster and smaller, and make more cases to be handled as "simple common cases".

Note that this change uses HAS_BUILTIN_{ADD,SUB,MUL}_OVERFLOW macros which have already been defined in config.h but seem not to have been used by existing code.

The behavior should be the same before and after this change.


  • This set of changes requires a perldelta entry, and it is included.

…vailable

This will hopefully make the code faster and smaller, and
make more cases to be handled as "simple common cases".

Note that this change uses HAS_BUILTIN_{ADD,SUB,MUL}_OVERFLOW macros
which have already been defined in config.h but seem not to have been
used by existing code.

t/op/64bitint.t: Add tests to exercise "simple common cases".
Note that these tests should pass even before this change.
@tonycoz
Copy link
Contributor

tonycoz commented Jul 30, 2025

This breaks Win32, which doesn't enable the __builtin_add_... etc builtins even for gcc.

I suspect it's due to long being 32-bits even on 64-bit Win32, but I haven't tried to debug it.

… in UV

If C compiler doesn't know __builtin_mul_overflow, S_uv_mul_overflow
will be implemented with fallback "long multiplication" algorithm,
but it had a bug that elemental multiplications were done in unsigned
long precision instead of UV precision.  It will lead wrong result
when unsigned long is narrower than UV (for example -Duse64bitint
on 32-bit platform).
@t-a-k
Copy link
Contributor Author

t-a-k commented Jul 31, 2025

This breaks Win32, which doesn't enable the __builtin_add_... etc builtins even for gcc.

I suspect it's due to long being 32-bits even on 64-bit Win32, but I haven't tried to debug it.

Thank you for your comment. My patch has a fallback code (similar to the code used before this change) for compilers with no overflow-checking builtins, but it had a bug (I was able to reproduce similar symptoms by ./Configure ... -Duse64bitint -Ud_builtin_mul_overflow on 32-bit x86 Linux). I've pushed a commit to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants