This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Don't use PyLong_SHIFT with _PyLong_AsScaledDouble()
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.2, Python 2.7
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: mark.dickinson, vstinner
Priority: normal Keywords: patch

Created on 2009-03-27 00:23 by vstinner, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
pylong_asscaleddouble-2.patch vstinner, 2009-03-27 00:23
issue5576.patch mark.dickinson, 2009-12-29 19:10
Messages (6)
msg84236 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2009-03-27 00:23
_PyLong_AsScaledDouble() writes the exponent in a int which have to be 
multiplied by PyLong_SHIFT to give the power of 2. I proposed to 
multiply the exponent by PyLong_SHIFT in _PyLong_AsScaledDouble() to 
directly get the power of 2. It avoids complex code to test integer 
overflow in code using _PyLong_AsScaledDouble() (the test is only done 
once, in _PyLong_AsScaledDouble).

I also propose to change exponent type from "int*" to "unsigned int*". 
Previous maximum exponent was INT_MAX//PyLong_SHIFT (eg. 143165576 for 
PyLong using base 2^15). The new maximum is now 
UINT_MAX//PyLong_SHIFT, the double ;-)

And since issue #4258 is commited (Use 30-bit digits instead of 15-bit 
digits for Python integers), PyLong_SHIFT value may be different 
depending on the compiler option. Using my patch, the long implement 
will be a little bit more hidden.

The function _PyLong_AsScaledDouble() should be private because the 
name starts with "_". But I see it in Include/longobject.h. In Python, 
it's used in longobject.c and mathmodule.c.
msg84314 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-28 16:41
Thanks for this.  I'll take a look.

How about making the exponent type size_t or Py_ssize_t, rather than 
[unsigned] int?  It seems to me that that fits better with the usual size 
in digits.
msg84372 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2009-03-29 10:52
Long double (80 bits) exponent is in range [-16382; 16383] and so would
fits in an int, unsigned int, size_t or Py_ssize_t. I don't know if a
signed or unsigned number is better. I know only one operation on
exponents: a-b in PyLong_AsDouble.
msg96998 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-12-29 18:15
> Long double (80 bits) exponent is in range [-16382; 16383] and so 
would
> fits in an int, unsigned int, size_t or Py_ssize_t.

Sure, but I don't think that's relevant to the point I was attempting to 
make:  PyLong_AsScaledDouble returns the number of bits in the given 
PyLong, and that number can be (theoretically) as large as 
PY_SSIZE_T_MAX * PyLong_SHIFT.  So Py_ssize_t is a better fit than int 
to hold this number:  otherwise you'll get unnecessary overflows on a 
64-bit system.

I'm working on refactoring PyLong_AsScaledDouble and PyLong_AsDouble to 
have them both use the same core code.  This would make 
PyLong_AsScaledDouble correctly rounded, and hence independent of the 
implementation of a PyLong (in particular, independent of the decision 
to use 15-bit or 30-bit digits for PyLongs: this decision really 
shouldn't affect the result of a log function or any other functions 
using _PyLong_AsScaledDouble).
msg96999 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-12-29 19:10
Here's a patch:

 - rename _PyLong_AsScaledDouble to _PyLong_Frexp (for the benefit of
   anyone foolish enough to be using an undocumented private API
   function)
 - make the exponent type Py_ssize_t instead of int
 - do the scaling by PyLong_SHIFT in _PyLong_Frexp instead of in the
   functions that call it (e.g., loghelper in the math module)
 - remove longintrepr.h include from math module, since it's no longer
   needed
 - change _PyLong_Frexp algorithm to do correct rounding
 - refactor PyLong_Double to use _PyLong_Frexp.
msg97136 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-01-02 15:35
Applied in r77234 (trunk), r77237 (py3k).
History
Date User Action Args
2022-04-11 14:56:47adminsetgithub: 49826
2010-01-02 15:35:55mark.dickinsonsetstatus: open -> closed
versions: + Python 3.2, - Python 3.1
messages: + msg97136

resolution: accepted
stage: patch review -> resolved
2009-12-29 19:10:56mark.dickinsonsetfiles: + issue5576.patch

messages: + msg96999
2009-12-29 18:15:20mark.dickinsonsetmessages: + msg96998
2009-03-29 10:52:02vstinnersetmessages: + msg84372
2009-03-28 16:43:23mark.dickinsonsetpriority: normal
assignee: mark.dickinson
type: enhancement
stage: patch review
2009-03-28 16:41:12mark.dickinsonsetmessages: + msg84314
2009-03-27 00:23:54vstinnersetnosy: + mark.dickinson
2009-03-27 00:23:21vstinnercreate