Skip to content

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

Merged
charris merged 3 commits intonumpy:maintenance/1.22.xfrom
charris:backport-21154
Mar 31, 2022
Merged

ENH, BLD: Fix math feature detection for wasm#21277
charris merged 3 commits intonumpy:maintenance/1.22.xfrom
charris:backport-21154

Conversation

@charris
Copy link
Copy Markdown
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.

2 participants