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

Some functions in pymath.c should be moved elsewhere. #51767

Closed
mdickinson opened this issue Dec 15, 2009 · 9 comments
Closed

Some functions in pymath.c should be moved elsewhere. #51767

mdickinson opened this issue Dec 15, 2009 · 9 comments
Assignees
Labels
build The build process and cross-build

Comments

@mdickinson
Copy link
Member

BPO 7518
Nosy @mdickinson, @vadmium

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/mdickinson'
closed_at = <Date 2009-12-21.15:28:10.764>
created_at = <Date 2009-12-15.19:19:34.713>
labels = ['build']
title = 'Some functions in pymath.c should be moved elsewhere.'
updated_at = <Date 2015-06-17.17:04:33.186>
user = 'https://github.com/mdickinson'

bugs.python.org fields:

activity = <Date 2015-06-17.17:04:33.186>
actor = 'mark.dickinson'
assignee = 'mark.dickinson'
closed = True
closed_date = <Date 2009-12-21.15:28:10.764>
closer = 'mark.dickinson'
components = ['Build']
creation = <Date 2009-12-15.19:19:34.713>
creator = 'mark.dickinson'
dependencies = []
files = []
hgrepos = []
issue_num = 7518
keywords = []
message_count = 9.0
messages = ['96455', '96464', '96499', '96500', '96502', '96507', '96754', '245430', '245443']
nosy_count = 3.0
nosy_names = ['mark.dickinson', 'rpetrov', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue7518'
versions = ['Python 2.7', 'Python 3.2']

@mdickinson
Copy link
Member Author

The Python/pymath.c file currently defines substitutes for some C99 libm
functions (e.g., log1p), for the benefit of platforms that don't implement
those functions.
It's compiled into the main Python executable.

There are (at least) two problems with this approach:

(1) on a platform that doesn't have (for example) log1p, we end up
exporting the symbol _log1p from the main Python executable. At the very
least, this should have a _Py prefix. Moreover, since log1p is only
needed by the math and cmath modules, and not by the Python interpreter
itself, python.exe shouldn't really be exporting it in the first place.

(2) It may not work! As an experiment, I tried adding a new function
expm1_alt to pymath.c, declaring it in Include/pymath.h with PyAPI_FUNC,
and using it in Modules/mathmodule.c; but the function ended up not being
visible to the math module: the math module failed to build, with an
error message:

dlopen(build/lib.macosx-10.4-i386-2.7/math.so, 2): Symbol not found:
_expm1_alt

When I moved the function to a different source file (I picked
Objects/longobject.c, for no particularly good reason) the new function
became visible and the build succeeded.

The reason for the above behaviour appears to be (I'm guessing a little
bit) that on my platform (OS X 10.6), none of the functions from pymath.c
is needed in the main python executable, so when creating python.exe the
linker ignores *all* the symbols from pymath.c. So while libpython2.7.a
exports _expm1_alt, python.exe does not.

I'm not sure what the best way to reorganize this setup is.

@mdickinson mdickinson added the build The build process and cross-build label Dec 15, 2009
@mdickinson
Copy link
Member Author

Incidentally, the build failure described earlier can be fixed by creating python.exe with:

gcc -u _PyMac_Error -o python.exe Modules/python.o -ldl -Wl,-force_load,libpython2.7.a -framework CoreFoundation

instead of with:

gcc -u _PyMac_Error -o python.exe Modules/python.o libpython2.7.a -ldl -framework CoreFoundation

@rpetrov
Copy link
Mannequin

rpetrov mannequin commented Dec 16, 2009

In general those functions has to be part of, lets call it "python
runtime/portable interface" . With current build system you may create a
build-in module lets call it mathport and to add some functions .
To the list a will add function atan2 from math module. I don't know why
the test cases for cmath pass on platforms with non-expected behaviour
of atan2-may be build boots lack those platforms. To me is expected for
all platforms on which "tanh preserves the sign of zero" configure test
case fail atan2 to fail too and cmath functions that use atan2 to fail.

@mdickinson
Copy link
Member Author

For the recently added expm1 function (see issue bpo-3366), I've put the
substitute code in a new file Modules/_math.c, that's linked into math.so.
I plan to move the other substitute libm functions from pymath.c into this
file, and to link _math.c into the cmath module as well as the math
module.

Roumen, I'm not sure I understand your comments about atan2. Is there
some link between atan2 and tanh that I'm not aware of? Which platforms
have unexpected behaviour for atan2?

@rpetrov
Copy link
Mannequin

rpetrov mannequin commented Dec 17, 2009

May be is good to add depends=['_math.h'], for modules in setup.py.

About atan2 - lets see comments in mathmodule and configure test. The
cmath test case pass on freebsd as buildbot is for for 7.2. What about
freebsd 6.2 ?
Also I don't think that python cmath tests will pass with MSVC before
8.0. It is based on my tests in the past with different MSVC runtimes.
In my notes for msvc 7.0.2600 result of 2.*atan2(-0,1.4142135623730951)
is 0(zero) instead -0.

@mdickinson
Copy link
Member Author

So the configure test for tanh(-0.0) is purely informational: it doesn't
affect the behaviour of the built Python in any way. I agree that the
test is imperfect, in that if atan2 is also non-conformant on the given
platform then the sign issue with tanh may go undetected, but all that
happens in that case is that we get a bogus 'preserves sign of -0 ... yes'
message in the configure output.

The wrapper for atan2 in Modules/mathmodules.c is a separate issue; it
should ensure that we get the right behaviour for atan2 on all platforms,
including msvc 7.0. I can't immediately see any reason why it wouldn't be
working as intended.

I don't think that python cmath tests will pass with MSVC before
8.0.

Which test(s) do you think will fail?

May be is good to add depends=['_math.h'], for modules in setup.py.

Good point---thanks! Done in r76865 (trunk), r76867 (py3k).

@mdickinson
Copy link
Member Author

Moved atanh, asinh, acosh, log1p from pymath.c to Modules/_math.c in
r76978 (trunk), r76980 (py3k).

@mdickinson mdickinson self-assigned this Dec 21, 2009
@vadmium
Copy link
Member

vadmium commented Jun 17, 2015

I suspect the change here causes _math.c to be compiled to the same object file twice, and there is a race condition when the compilations are concurrent. See bpo-24421.

@mdickinson
Copy link
Member Author

Yes, there are almost certainly better ways to organize the build here.

A hacky solution might be to simply #include _math.c in the two other files that need it.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build The build process and cross-build
Projects
None yet
Development

No branches or pull requests

2 participants