Re: bignum branch

[Andy Moreton <andrewjmoreton@gmail.com>]

On Fri 03 Aug 2018, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Fri, 03 Aug 2018 10:01:59 +0100
>> 
>> > I believe your changes also cover the 32-bit MinGW build with wide ints.
>> 
>> I expect so, but the build fails for 32bit MinGW builds on bignum branch
>> with gettimeofday problems. I have a feeling that this has already been
>> fixed on the master branch some time ago.
>
> Could be, but can you show the error messages?

On the bignum ranch for 32bit MinGW I get:

make -C lib all
make[1]: Entering directory `/c/emacs/git/emacs/bignum/build/mingw32/lib'
  CC       gettimeofday.o
In file included from c:/emacs/git/emacs/bignum/lib/gettimeofday.c:23:0:
c:/emacs/git/emacs/bignum/nt/inc/sys/time.h:11:19: error: field 'it_interval' has incomplete type
   struct  timeval it_interval; /* timer interval */
                   ^~~~~~~~~~~
c:/emacs/git/emacs/bignum/nt/inc/sys/time.h:12:19: error: field 'it_value' has incomplete type
   struct  timeval it_value; /* current value */
                   ^~~~~~~~
c:/emacs/git/emacs/bignum/lib/gettimeofday.c:64:1: error: conflicting types for 'gettimeofday'
 gettimeofday (struct timeval *restrict tv, void *restrict tz)
 ^~~~~~~~~~~~
In file included from c:/emacs/git/emacs/bignum/nt/inc/sys/time.h:4:0,
                 from c:/emacs/git/emacs/bignum/lib/gettimeofday.c:23:
c:\mingw\include\sys\time.h:108:29: note: previous declaration of 'gettimeofday' was here
 int __cdecl __MINGW_NOTHROW gettimeofday
                             ^~~~~~~~~~~~
c:/emacs/git/emacs/bignum/lib/gettimeofday.c: In function 'gettimeofday':
c:/emacs/git/emacs/bignum/lib/gettimeofday.c:100:5: error: dereferencing pointer to incomplete type 'struct timeval'
   tv->tv_sec = microseconds_since_1970 / (ULONGLONG) 1000000;
     ^~
make[1]: *** [gettimeofday.o] Error 1
make[1]: Leaving directory `/c/emacs/git/emacs/bignum/build/mingw32/lib'
make: *** [lib] Error 2


>> > Can you tell what is the following hunk about?
>> >
>> >> @@ -3414,7 +3473,7 @@ Markers are converted to integers.  */)
>> >>    else
>> >>      {
>> >>        eassume (FIXNUMP (number));
>> >> -      if (XINT (number) > MOST_POSITIVE_FIXNUM)
>> >> +      if (XINT (number) > MOST_NEGATIVE_FIXNUM)
>> >>  	XSETINT (number, XINT (number) - 1);
>> >>        else
>> >>  	{
>> 
>> The updated Fadd1 checks if the old value is MOST_POSITIVE_FIXNUM before
>> incrementing, as that would need a bignum result.
>> 
>> This hunk in Fsub1 should check if the old value is MOST_NEGATIVE_FIXNUM
>> before decrementing, as that would need a bignum result.
>
> So the previous code in Fsub1 had a bug?

The code on master is fine, but there was a bug in the conversion to
bignums. It looks like the code in Fsub1 was copy-pasted from Fadd1
but not all of the differences got fixed up.

>
>> > Also, this:
>> >
>> >> +(defun ccl-fixnum (code)
>> >> +  "Convert a CCL code word to a fixnum value."
>> >> +  (- (logxor (logand code #x0fffffff) #x08000000) #x08000000))
>> 
>> The CCL compiled codewords are 28bits, but the CCL implementation
>> assumes that the codewords are sign-extended, so that data constants in
>> the upper part of the codeword are signed. This function truncates a
>> codeword to 28bits, and then sign extends the result to a fixnum.
>> 
>> This ensures that the CCL compiler output is the same on master and
>> bignum branches.
>
> Yes, I know.  I was asking for a comment to that effect, so that
> others won't wonder what this is about.

I'll update the patch to add a comment.

    AndyM