Skip to content
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

ENH, BLD: Fix math feature detection for wasm #21277

Merged
merged 3 commits into from Mar 31, 2022

Conversation

charris
Copy link
Member

@charris charris commented Mar 31, 2022

Backport of #21254.

The web assembly linker is strict about function types, it is
unwilling to link a function defined with signature say double
f(double, double) to an invocation with signature void f(void). This
causes trouble in numpy/core/setup.py in the functions
check_math_capabilities and check_mathlib.

This patch fixes the problem by giving config.try_link the correct
function signatures for these functions. In particular I added a
separate header file with all the math declarations. It would be be
possible to just include 'math.h' but we also need to parse the
declarations to figure out how to call the functions. If f has
arguments type1, type2, type3, we want to call it like f((type1)0,
(type2)0, (type3)0). Anyways it is easier to parse the arguments out
of our feature_detection_math.h than to worry about the possibility
that someone will have a differently formatted system math.h. We do a
test where we include both math.h and feature_detection_math.h to
ensure consistency between the signatures.

I also separated out the fcntl functions, backtrace and madvise, and
strtold_l. This is because they require separate headers. strtold_l
requires testing both with the locale.h header and with the xlocale.h
header (this seems to vary even among different linuxes). All of the
functions I moved out of OPTIONAL_STDFUNCS are absent on windows and
some are absent on OSX so separating them out of the
OPTIONAL_STDFUNCS should mildly improve the speed of feature
detection on mac and windows (since they have all the functions
remaining in OPTIONAL_STDFUNCS).

The web assembly linker is strict about function types, it is unwilling
to link a function defined with signature say `double f(double, double)`
to an invocation with signature `void f(void)`. This causes trouble in
`numpy/core/setup.py` in the functions `check_math_capabilities` and
`check_mathlib`.

This patch fixes the problem by giving config.try_link the correct
function signatures for these functions. In particular I added a separate
header file with all the math declarations. It would be be possible to
just include 'math.h' but we also need to parse the declarations to figure
out how to call the functions. If f has arguments type1, type2, type3, we
want to call it like f((type1)0, (type2)0, (type3)0). Anyways it is easier
to parse the arguments out of our feature_detection_math.h than to worry
about the possibility that someone will have a differently formatted system
math.h. We do a test where we include both math.h and feature_detection_math.h
to ensure consistency between the signatures.

I also separated out the fcntl functions, backtrace and madvise, and strtold_l.
This is because they require separate headers. strtold_l requires testing both
with the locale.h header and with the xlocale.h header (this seems to vary even
among different linuxes). All of the functions I moved out of OPTIONAL_STDFUNCS
are absent on windows and some are absent on OSX so separating them out of the
OPTIONAL_STDFUNCS should mildly improve the speed of feature detection on mac
and windows (since they have all the functions remaining in OPTIONAL_STDFUNCS).
@charris charris added 01 - Enhancement 08 - Backport Used to tag backport PRs 36 - Build Build related PR labels Mar 31, 2022
@charris charris added this to the 1.22.4 release milestone Mar 31, 2022
@charris charris merged commit 966e7a7 into numpy:maintenance/1.22.x Mar 31, 2022
@charris charris deleted the backport-21154 branch April 10, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
01 - Enhancement 08 - Backport Used to tag backport PRs 36 - Build Build related PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants