Re: 64-bit integer arithmetics v2

[Jaroslav Hajek]

On Mon, Oct 6, 2008 at 8:15 PM, Jaroslav Hajek <address@hidden> wrote:
> On Mon, Oct 6, 2008 at 7:55 PM, John W. Eaton <address@hidden> wrote:
>> On  5-Oct-2008, Jaroslav Hajek wrote:
>>
>> | this is an improved version of the previous attempt at implementing
>> | 64-bit arithmetics.
>> | The mixed precision operators and relations should now work robustly
>> | in all cases (and in case you have long double, they will even be fast
>> | :)
>> | These expressions should now work as expected:
>> | uint64(2^64), int64(2^63)...
>> | a = 1.9*2^61; b = uint64(a); b++; b > a
>> | a = 1.9*2^61; b = int64(a); b++; b > a
>> | (note that these two may already work in current tip and/or 3.0.x
>> | because of the CPU using long double behind the scene, but it will
>> | probably depend on optimization level) and maybe more ... (tests yet
>> | to be written).
>>
>> OK.
>>
>> | also, the mixed (u)int64-double binary operations avoid precision loss
>> | (using long double if possible).
>> |
>> | OK to push?
>>
>> Yes, but before doing that, will you please fix the following?
>>
>> | +#include <math.h>
>>
>> Octave C++ files should not include standard C headers directly.
>> Instead of <math.h>, we should be including <cmath>.
>>
>
> OK
>

I included "lo-math.h" instead of math.h.

>> | +#if (SIZEOF_LONG_DOUBLE >= 10) && defined (HAVE_ROUNDL)
>> | +inline long double xround (long double x) { return roundl (x); }
>> | +#define OCTAVE_INT_USE_LONG_DOUBLE
>> | +#endif
>>
>> The test for SIZEOF_LONG_DOUBLE probably works in practice, but the
>> overall size doesn't guarantee the precision, does it?  So it would
>> probably be better to have a test that checks whether a 64-bit integer
>> can be exactly represented by the long double type.
>>
>> How about changing the macro to something like
>> OCTAVE_INT64_FITS_IN_LONG_DOUBLE (I'm not sure of the best name) and
>> then defining that in config.h if SIZEOF_LONG_DOUBLE is >= 10, and
>> also inserting a comment in configure.in saying that it would be
>> better to have a proper test for this rather than just checking the
>> size?
>>
>
> I thought about this, initially; but is it really necessary?
> The 80-bit floating point format (extended double) is an IEEE
> standard, so it is unlikely that an 80-bits wide long double would not
> comply to this.
> Since I intend to supply checks for the corner cases in int64
> arithmetics, they will alert us if Octave is ever ported to a non-IEEE
> platform (actually, I think that a couple more parts of Octave will
> work improperly with non-IEEE floats).
> But anyway, the suggestion to move the
> #if (SIZEOF_LONG_DOUBLE >= 10) && defined (HAVE_ROUNDL)
> clause to config.h and only use OCTAVE_INT_USE_LONG_DOUBLE
> in oct-inttypes.h is certainly a good idea.
>

The OCTAVE_INT_USE_LONG_DOUBLE is now defined in config.h
(and can later be replaced with a more sophisticated check)

>
>> Also, it would be good to avoid these warnings:
>>
>>  /home/jwe/src/octave/liboctave/oct-inttypes.h: In static member function 
>> 'static T octave_int_base<T>::convert_real(const S&) [with S = long double, 
>> T = long int]':
>>  /home/jwe/src/octave/liboctave/oct-inttypes.h:768:   instantiated from 
>> 'octave_int<T>::octave_int(long double) [with T = long int]'
>>  /home/jwe/src/octave/liboctave/oct-inttypes.h:1022:   instantiated from here
>>  /home/jwe/src/octave/liboctave/oct-inttypes.h:287: warning: unused variable 
>> 'thmin'
>>  /home/jwe/src/octave/liboctave/oct-inttypes.h:288: warning: unused variable 
>> 'thmax'
>>
>> Since oct-inttypes.h is included in many files, these are repeated
>> many times.
>>
>
> I'll look at this.
>

These were bugs! Thanks. It seems warnings are not useless after all :)


The updated changeset is attached. OK to push?


-- 
RNDr. Jaroslav Hajek
computing expert
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz

int_patch2.diff
Description: Text Data