msg412531 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-04 20:24 |
While debugging a GCC regression (*) on "HUGE_VAL * 0" used by Py_NAN macro, I noticed that Python can now C99 "NAN" constant.
(*) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104389
In bpo-45440, I already removed legacy code for pre-C99 support and old platforms:
"Building Python now requires a C99 <math.h> header file providing the following functions: copysign(), hypot(), isfinite(), isinf(), isnan(), round()."
Attached patch modifies Py_NAN to simply reuse NAN.
mathmodule.c and cmathmodule.c m_nan() still use _Py_dg_stdnan() by default (if PY_NO_SHORT_FLOAT_REPR is not defined).
|
msg412533 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-04 20:30 |
The Py_NAN has a special implementation for the ICC compiler, if __INTEL_COMPILER and ICC_NAN_STRICT macros are defined, in bpo-21167:
---
commit edbc28ce81f46d042f9d5ddf9c5bc8cecebc715a
Author: R David Murray <rdmurray@bitdance.com>
Date: Thu Aug 13 09:58:07 2015 -0400
#21167: Fix definition of NAN when ICC used without -fp-model strict.
Patch from Chris Hogan of Intel, reviewed by Mark Dickinson.
---
I don't know if it should be kept if Py_NAN is modified to use the NAN constant. In case of doubt, I prefer to remove the ICC code since we have no ICC buildbot anymore and maybe ICC changed in the meanwhile.
Also, I don't have acess to ICC.
|
msg412536 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-04 20:47 |
> mathmodule.c and cmathmodule.c m_nan() still use _Py_dg_stdnan() by default (if PY_NO_SHORT_FLOAT_REPR is not defined).
These functions are only use to create the following constants:
* math.nan
* cmath.nan
* cmath.nanj
|
msg412538 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-04 20:55 |
Manual test to check if m_nan(), _Py_dg_stdnan(0) and Py_NAN are exactly the same number (same bits):
$ ./python
>>> import math, struct
>>> m_nan=math.nan; Py_NAN=math.atan2(m_nan, 1.0)
>>> Py_NAN is m_nan
False
>>> struct.pack('d', m_nan) == struct.pack('d', Py_NAN)
True
>>> struct.pack('d', Py_NAN)
b'\x00\x00\x00\x00\x00\x00\xf8\x7f'
=> see attached script: test_nan_bits.py
"struct.pack('d', m_nan) == struct.pack('d', Py_NAN)" is true with GH-31134 on Fedora 35 with gcc-11.2.1-7.fc35.x86_64. I tested with "gcc -O0" and "gcc -O3".
GCC float.h defines NAN with:
#define NAN (__builtin_nanf (""))
GCC: "Built-in Function: double __builtin_nan (const char *str): This is an implementation of the ISO C99 function nan."
|
msg412539 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-04 20:57 |
Python/dtoa.c uses:
/* Standard NaN used by _Py_dg_stdnan. */
#define NAN_WORD0 0x7ff80000
#define NAN_WORD1 0
/* Return a 'standard' NaN value.
There are exactly two quiet NaNs that don't arise by 'quieting' signaling
NaNs (see IEEE 754-2008, section 6.2.1). If sign == 0, return the one whose
sign bit is cleared. Otherwise, return the one whose sign bit is set.
*/
double
_Py_dg_stdnan(int sign)
{
U rv;
word0(&rv) = NAN_WORD0;
word1(&rv) = NAN_WORD1;
if (sign)
word0(&rv) |= Sign_bit;
return dval(&rv);
}
|
msg412540 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-04 21:14 |
> see attached script: test_nan_bits.py
test_nan_bits.py says "True" for Python built with "clang -O3" and with "clang -O0".
|
msg412542 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-04 21:15 |
Using clang -E, I see that clang also replaces NAN with: __builtin_nanf ("").
|
msg412576 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2022-02-05 15:08 |
The big blocker here is that a platform that fully supports C99 might not define the "NAN" macro. I don't think we can require that NAN be defined in order for Python to build (which is what the PR currently does, if I'm understanding it correctly).
Python deliberately doesn't assume IEEE 754 floating-point. By requiring that the C "NAN" macro is present to be able to build Python, we'd be effectively requiring IEEE 754 by stealth. (No other common floating-point format has NaNs.)
I'd be fully on board with a decision to require IEEE 754 floating-point for Python in future, but that decision would at least need a python-dev discussion - we shouldn't sneak it in by the back door.
|
msg412581 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-05 16:42 |
> The big blocker here is that a platform that fully supports C99 might not define the "NAN" macro. I don't think we can require that NAN be defined in order for Python to build (which is what the PR currently does, if I'm understanding it correctly).
If a platform doesn't implement NaN, it should define the Py_NO_NAN macro:
/* Py_NAN
* A value that evaluates to a NaN. On IEEE 754 platforms INF*0 or
* INF/INF works. Define Py_NO_NAN in pyconfig.h if your platform
* doesn't support NaNs.
*/
#if !defined(Py_NAN) && !defined(Py_NO_NAN)
// Use C99 "NAN" constant: quiet Not-A-Number (when supported)
# define Py_NAN NAN
#endif
Or do you mean that a platform can support NaN but don't define the <math.h> NAN macro?
|
msg412582 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2022-02-05 16:47 |
> If a platform doesn't implement NaN, it should define the Py_NO_NAN macro
Ah. In that case your PR description (and the PR news entry) is misleading:
> Building Python now requires a C99 <math.h> header file providing the
> NAN constant.
Please could you update them?
|
msg412624 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-06 12:13 |
New changeset 54842e4311bb0e34012d1984b42eab41eeeaea6a by Victor Stinner in branch 'main':
bpo-46640: Py_NAN now uses the C99 NAN constant (GH-31134)
https://github.com/python/cpython/commit/54842e4311bb0e34012d1984b42eab41eeeaea6a
|
msg412626 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-06 12:30 |
I merged my change, thanks for the reviews.
|
msg412728 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2022-02-07 09:48 |
Adding new C99 features needs a change in PEP 7 (https://www.python.org/dev/peps/pep-0007/#c-dialect)
|
msg412743 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-07 13:42 |
> Adding new C99 features needs a change in PEP 7 (https://www.python.org/dev/peps/pep-0007/#c-dialect)
IMO this PEP is outdated for a long time.
C99 standard is wide. Do we have to explicitly list every single function, macro or constant used by Python? It doesn't sound reasonable to me. IMO saying that we use "C99 except of these few features: <...>" would be closer to the reality. I don't know which features are not used.
Well, if you ask me, I would simply require a C99 compiler. That's all :-)
Note: Python uses C11 <stdatomic.h>, but it remains an optional requirement.
|
msg412747 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-07 14:18 |
> Well, if you ask me, I would simply require a C99 compiler. That's all :-)
Done in https://github.com/python/peps/pull/2309
|
msg412748 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2022-02-07 14:18 |
> IMO this PEP is outdated for a long time.
It is not. Even if it is, it should be marked as such, and that is not a decision that should be done in this issue.
Please, don't break the rules because you think they're outdated.
> Well, if you ask me, I would simply require a C99 compiler. That's all :-)
Please propose that change. Perhaps it would be a good change to make, but I don't even know how to determine that.
Nor can I list the places where the change should be made -- at least there should be a What's New entry like https://docs.python.org/3.10/whatsnew/3.6.html#build-and-c-api-changes
> Note: Python uses C11 <stdatomic.h>, but it remains an optional requirement.
That's fine. You can still build with an older compiler.
|
msg414002 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2022-02-25 15:09 |
PEP 7 has been updated, I close the issue.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:55 | admin | set | github: 90798 |
2022-02-25 15:09:43 | vstinner | set | status: open -> closed
messages:
+ msg414002 |
2022-02-07 14:18:49 | petr.viktorin | set | messages:
+ msg412748 |
2022-02-07 14:18:29 | vstinner | set | messages:
+ msg412747 |
2022-02-07 13:42:02 | vstinner | set | messages:
+ msg412743 |
2022-02-07 09:48:44 | petr.viktorin | set | status: closed -> open nosy:
+ petr.viktorin messages:
+ msg412728
|
2022-02-06 12:30:09 | vstinner | set | status: open -> closed title: Python can now use the C99 NAN constant -> Python can now use the C99 NAN constant or __builtin_nan() messages:
+ msg412626
resolution: fixed stage: patch review -> resolved |
2022-02-06 12:13:17 | vstinner | set | messages:
+ msg412624 |
2022-02-05 16:47:14 | mark.dickinson | set | messages:
+ msg412582 |
2022-02-05 16:42:34 | vstinner | set | messages:
+ msg412581 |
2022-02-05 15:08:10 | mark.dickinson | set | nosy:
+ tim.peters, rhettinger messages:
+ msg412576
|
2022-02-04 21:15:08 | vstinner | set | messages:
+ msg412542 |
2022-02-04 21:14:19 | vstinner | set | messages:
+ msg412540 |
2022-02-04 20:57:28 | vstinner | set | messages:
+ msg412539 |
2022-02-04 20:55:44 | vstinner | set | files:
+ test_nan_bits.py
messages:
+ msg412538 |
2022-02-04 20:47:15 | vstinner | set | messages:
+ msg412536 |
2022-02-04 20:30:48 | vstinner | set | nosy:
+ mark.dickinson, r.david.murray messages:
+ msg412533
|
2022-02-04 20:28:10 | vstinner | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request29313 |
2022-02-04 20:24:09 | vstinner | create | |