From f2119f95b505b31cdfbf66e431544036238c8c68 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Sat, 5 Mar 2022 16:00:54 -0800 Subject: [PATCH 1/3] MAINT, BLD Fix math feature detection for wasm 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). --- numpy/core/feature_detection_locale.h | 1 + numpy/core/feature_detection_math.h | 107 ++++++++++++++++++++++++++ numpy/core/feature_detection_misc.h | 4 + numpy/core/feature_detection_stdio.h | 6 ++ numpy/core/setup.py | 79 ++++++++++++++----- numpy/core/setup_common.py | 39 ++++++++-- 6 files changed, 213 insertions(+), 23 deletions(-) create mode 100644 numpy/core/feature_detection_locale.h create mode 100644 numpy/core/feature_detection_math.h create mode 100644 numpy/core/feature_detection_misc.h create mode 100644 numpy/core/feature_detection_stdio.h diff --git a/numpy/core/feature_detection_locale.h b/numpy/core/feature_detection_locale.h new file mode 100644 index 000000000000..0af1d6e7e640 --- /dev/null +++ b/numpy/core/feature_detection_locale.h @@ -0,0 +1 @@ +long double strtold_l(const char*, char**, locale_t); diff --git a/numpy/core/feature_detection_math.h b/numpy/core/feature_detection_math.h new file mode 100644 index 000000000000..27ad7bcf0139 --- /dev/null +++ b/numpy/core/feature_detection_math.h @@ -0,0 +1,107 @@ +double expm1(double); +double log1p(double); +double acosh(double); +double asinh(double); +double atanh(double); +double rint(double); +double trunc(double); +double exp2(double); +double log2(double); +double hypot(double, double); +double atan2(double, double); +double pow(double, double); +double copysign(double, double); +double nextafter(double, double); +long long strtoll(const char*, char**, int); +unsigned long long strtoull(const char*, char**, int); +double cbrt(double); +long double sinl(long double); +long double cosl(long double); +long double tanl(long double); +long double sinhl(long double); +long double coshl(long double); +long double tanhl(long double); +long double fabsl(long double); +long double floorl(long double); +long double ceill(long double); +long double rintl(long double); +long double truncl(long double); +long double sqrtl(long double); +long double log10l(long double); +long double logl(long double); +long double log1pl(long double); +long double expl(long double); +long double expm1l(long double); +long double asinl(long double); +long double acosl(long double); +long double atanl(long double); +long double asinhl(long double); +long double acoshl(long double); +long double atanhl(long double); +long double hypotl(long double, long double); +long double atan2l(long double, long double); +long double powl(long double, long double); +long double fmodl(long double, long double); +long double modfl(long double, long double*); +long double frexpl(long double, int*); +long double ldexpl(long double, int); +long double exp2l(long double); +long double log2l(long double); +long double copysignl(long double, long double); +long double nextafterl(long double, long double); +long double cbrtl(long double); +float sinf(float); +float cosf(float); +float tanf(float); +float sinhf(float); +float coshf(float); +float tanhf(float); +float fabsf(float); +float floorf(float); +float ceilf(float); +float rintf(float); +float truncf(float); +float sqrtf(float); +float log10f(float); +float logf(float); +float log1pf(float); +float expf(float); +float expm1f(float); +float asinf(float); +float acosf(float); +float atanf(float); +float asinhf(float); +float acoshf(float); +float atanhf(float); +float hypotf(float, float); +float atan2f(float, float); +float powf(float, float); +float fmodf(float, float); +float modff(float, float*); +float frexpf(float, int*); +float ldexpf(float, int); +float exp2f(float); +float log2f(float); +float copysignf(float, float); +float nextafterf(float, float); +float cbrtf(float); +double sin(double); +double cos(double); +double tan(double); +double sinh(double); +double cosh(double); +double tanh(double); +double fabs(double); +double floor(double); +double ceil(double); +double sqrt(double); +double log10(double); +double log(double); +double exp(double); +double asin(double); +double acos(double); +double atan(double); +double fmod(double, double); +double modf(double, double*); +double frexp(double, int*); +double ldexp(double, int); diff --git a/numpy/core/feature_detection_misc.h b/numpy/core/feature_detection_misc.h new file mode 100644 index 000000000000..f58cf4b62e99 --- /dev/null +++ b/numpy/core/feature_detection_misc.h @@ -0,0 +1,4 @@ +#include + +int backtrace(void **, int); +int madvise(void *, size_t, int); diff --git a/numpy/core/feature_detection_stdio.h b/numpy/core/feature_detection_stdio.h new file mode 100644 index 000000000000..bc14d16d04ff --- /dev/null +++ b/numpy/core/feature_detection_stdio.h @@ -0,0 +1,6 @@ +#include +#include + +off_t ftello(FILE *stream); +int fseeko(FILE *stream, off_t offset, int whence); +int fallocate(int, int, off_t, off_t); diff --git a/numpy/core/setup.py b/numpy/core/setup.py index 7eb08132e2b2..136492dd5e86 100644 --- a/numpy/core/setup.py +++ b/numpy/core/setup.py @@ -125,32 +125,48 @@ def win32_checks(deflist): deflist.append('FORCE_NO_LONG_DOUBLE_FORMATTING') def check_math_capabilities(config, ext, moredefs, mathlibs): - def check_func(func_name): - return config.check_func(func_name, libraries=mathlibs, - decl=True, call=True) - - def check_funcs_once(funcs_name): - decl = dict([(f, True) for f in funcs_name]) - st = config.check_funcs_once(funcs_name, libraries=mathlibs, - decl=decl, call=decl) + def check_func( + func_name, + decl=False, + headers=["feature_detection_math.h"], + ): + return config.check_func( + func_name, + libraries=mathlibs, + decl=decl, + call=True, + call_args=FUNC_CALL_ARGS[func_name], + headers=headers, + ) + + def check_funcs_once(funcs_name, headers=["feature_detection_math.h"]): + call = dict([(f, True) for f in funcs_name]) + call_args = dict([(f, FUNC_CALL_ARGS[f]) for f in funcs_name]) + st = config.check_funcs_once( + funcs_name, + libraries=mathlibs, + decl=False, + call=call, + call_args=call_args, + headers=headers, + ) if st: moredefs.extend([(fname2def(f), 1) for f in funcs_name]) return st - def check_funcs(funcs_name): + def check_funcs(funcs_name, headers=["feature_detection_math.h"]): # Use check_funcs_once first, and if it does not work, test func per # func. Return success only if all the functions are available - if not check_funcs_once(funcs_name): + if not check_funcs_once(funcs_name, headers=headers): # Global check failed, check func per func for f in funcs_name: - if check_func(f): + if check_func(f, headers=headers): moredefs.append((fname2def(f), 1)) return 0 else: return 1 #use_msvc = config.check_decl("_MSC_VER") - if not check_funcs_once(MANDATORY_FUNCS): raise SystemError("One of the required function to build numpy is not" " available (the list is %s)." % str(MANDATORY_FUNCS)) @@ -165,15 +181,34 @@ def check_funcs(funcs_name): for f in OPTIONAL_STDFUNCS_MAYBE: if config.check_decl(fname2def(f), headers=["Python.h", "math.h"]): - OPTIONAL_STDFUNCS.remove(f) + if f in OPTIONAL_STDFUNCS: + OPTIONAL_STDFUNCS.remove(f) + else: + OPTIONAL_FILE_FUNCS.remove(f) + check_funcs(OPTIONAL_STDFUNCS) + check_funcs(OPTIONAL_FILE_FUNCS, headers=["feature_detection_stdio.h"]) + check_funcs(OPTIONAL_MISC_FUNCS, headers=["feature_detection_misc.h"]) + + for h in OPTIONAL_HEADERS: if config.check_func("", decl=False, call=False, headers=[h]): h = h.replace(".", "_").replace(os.path.sep, "_") moredefs.append((fname2def(h), 1)) + # Try with both "locale.h" and "xlocale.h" + locale_headers = [ + "stdlib.h", + "xlocale.h", + "feature_detection_locale.h", + ] + if not check_funcs(OPTIONAL_LOCALE_FUNCS, headers=locale_headers): + # It didn't work with xlocale.h, maybe it will work with locale.h? + locale_headers[1] = "locale.h" + check_funcs(OPTIONAL_LOCALE_FUNCS, headers=locale_headers) + for tup in OPTIONAL_INTRINSICS: headers = None if len(tup) == 2: @@ -394,20 +429,28 @@ def check_types(config_cmd, ext, build_dir): def check_mathlib(config_cmd): # Testing the C math library mathlibs = [] - mathlibs_choices = [[], ['m'], ['cpml']] - mathlib = os.environ.get('MATHLIB') + mathlibs_choices = [[], ["m"], ["cpml"]] + mathlib = os.environ.get("MATHLIB") if mathlib: - mathlibs_choices.insert(0, mathlib.split(',')) + mathlibs_choices.insert(0, mathlib.split(",")) for libs in mathlibs_choices: - if config_cmd.check_func("exp", libraries=libs, decl=True, call=True): + if config_cmd.check_func( + "log", + libraries=libs, + call_args="0", + decl="double log(double);", + call=True + ): mathlibs = libs break else: raise RuntimeError( "math library missing; rerun setup.py after setting the " - "MATHLIB env variable") + "MATHLIB env variable" + ) return mathlibs + def visibility_define(config): """Return the define value to use for NPY_VISIBILITY_HIDDEN (may be empty string).""" diff --git a/numpy/core/setup_common.py b/numpy/core/setup_common.py index 70e8fc89705a..181c58fb1219 100644 --- a/numpy/core/setup_common.py +++ b/numpy/core/setup_common.py @@ -1,8 +1,9 @@ # Code common to build tools -import sys -import warnings import copy +import pathlib +import sys import textwrap +import warnings from numpy.distutils.misc_util import mingw32 @@ -101,6 +102,32 @@ def check_api_version(apiversion, codegen_dir): warnings.warn(msg % (apiversion, curapi_hash, apiversion, api_hash, __file__), MismatchCAPIWarning, stacklevel=2) + + +FUNC_CALL_ARGS = {} + +def set_sig(sig): + prefix, _, args = sig.partition("(") + args = args.rpartition(")")[0] + funcname = prefix.rpartition(" ")[-1] + args = [arg.strip() for arg in args.split(",")] + FUNC_CALL_ARGS[funcname] = ", ".join("(%s) 0" % arg for arg in args) + + +for file in [ + "feature_detection_locale.h", + "feature_detection_math.h", + "feature_detection_misc.h", + "feature_detection_stdio.h", +]: + with open(pathlib.Path(__file__).parent / file) as f: + for line in f: + if line.startswith("#"): + continue + if not line.strip(): + continue + set_sig(line) + # Mandatory functions: if not found, fail the build MANDATORY_FUNCS = ["sin", "cos", "tan", "sinh", "cosh", "tanh", "fabs", "floor", "ceil", "sqrt", "log10", "log", "exp", "asin", @@ -110,9 +137,11 @@ def check_api_version(apiversion, codegen_dir): # replacement implementation. Note that some of these are C99 functions. OPTIONAL_STDFUNCS = ["expm1", "log1p", "acosh", "asinh", "atanh", "rint", "trunc", "exp2", "log2", "hypot", "atan2", "pow", - "copysign", "nextafter", "ftello", "fseeko", - "strtoll", "strtoull", "cbrt", "strtold_l", "fallocate", - "backtrace", "madvise"] + "copysign", "nextafter", "strtoll", "strtoull", "cbrt"] + +OPTIONAL_LOCALE_FUNCS = ["strtold_l"] +OPTIONAL_FILE_FUNCS = ["ftello", "fseeko", "fallocate"] +OPTIONAL_MISC_FUNCS = ["backtrace", "madvise"] OPTIONAL_HEADERS = [ From f1c10142020b958f2289db4c8d0e5a942c98f2f9 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Sat, 26 Mar 2022 16:37:43 -0700 Subject: [PATCH 2/3] Add upcoming_changes note --- .../upcoming_changes/21154.improvement.rst | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 doc/release/upcoming_changes/21154.improvement.rst diff --git a/doc/release/upcoming_changes/21154.improvement.rst b/doc/release/upcoming_changes/21154.improvement.rst new file mode 100644 index 000000000000..5ff2a3ebf979 --- /dev/null +++ b/doc/release/upcoming_changes/21154.improvement.rst @@ -0,0 +1,23 @@ +Math C library feature detection now works for wasm +--------------------------------------------------- +The previous feature detection generated C stubs like: + +.. code-block:: C + + void exp(void); // incorrect signature! + exp(); + +Then checked whether compiling and linking these worked. This generates warnings +that complain that ``exp`` actually has signature ``double exp(double)`` but would +otherwise work. However ``wasm-ld`` creates a hard error in this case, so when +compiling to a wasm target everything is detected as missing. + +This is now fixed: we feature detect instead with code like: + +.. code-block:: C + + double exp(double); // correct signature! + exp((double)0); + +which ``wasm-ld`` will link. As a side effect, the feature detection code now +works without generating linker warnings. From 543e135a6f9cbd5e88ae60085137675604ae9401 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 31 Mar 2022 08:06:17 -0700 Subject: [PATCH 3/3] Use description from mattip --- .../upcoming_changes/21154.improvement.rst | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/doc/release/upcoming_changes/21154.improvement.rst b/doc/release/upcoming_changes/21154.improvement.rst index 5ff2a3ebf979..38630b47b5da 100644 --- a/doc/release/upcoming_changes/21154.improvement.rst +++ b/doc/release/upcoming_changes/21154.improvement.rst @@ -1,23 +1,7 @@ -Math C library feature detection now works for wasm ---------------------------------------------------- -The previous feature detection generated C stubs like: - -.. code-block:: C - - void exp(void); // incorrect signature! - exp(); - -Then checked whether compiling and linking these worked. This generates warnings -that complain that ``exp`` actually has signature ``double exp(double)`` but would -otherwise work. However ``wasm-ld`` creates a hard error in this case, so when -compiling to a wasm target everything is detected as missing. - -This is now fixed: we feature detect instead with code like: - -.. code-block:: C - - double exp(double); // correct signature! - exp((double)0); - -which ``wasm-ld`` will link. As a side effect, the feature detection code now -works without generating linker warnings. +Math C library feature detection now uses correct signatures +------------------------------------------------------------ +Compiling is preceded by a detection phase to determine whether the +underlying libc supports certain math operations. Previously this code +did not respect the proper signatures. Fixing this enables compilation +for the ``wasm-ld`` backend (compilation for web assembly) and reduces +the number of warnings.