Re: bignum branch

[Andy Moreton]

On Fri 03 Aug 2018, Eli Zaretskii wrote:

>> From: Andy Moreton <address@hidden>
>> Date: Fri, 03 Aug 2018 01:43:17 +0100
>> 
>> After a lot more testing, I have a somewhat scruffy patch that works in
>> the following emacs builds on unpatched master, and on patched bignum branch:
>>  - cygwin 64bit (LP64 model)
>>  - mingw64 msys2 32bit
>>  - mingw64 msys2 64bit (LLP64 model)

I have now also tested
    - mingw64 msys2 32bit (wide int)

> 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.

>> The patch contains changes for:
>>  - fix CCL to ensure it uses 28biut codewords properly in bignum build
>>  - ensure make_number creates fixnums in LLP64 builds
>>  - fix bugnumcompare for LLP64 builds
>>  - fix arith_driver to handle markers correctly
>>  - fix arith_driver operations for LLP64 builds (more testing needed)
>>  - fix float_arith_driver to allow bignums
>>  - fix ash_lsh_impl to produce correct results in bignum build
>>  - fix NUMBERP to remove redundant BIGNUMP test (ensured by INTEGERP)
>
> 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.


> 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.

> Btw, isn't it confusing that INTEGERP allows fixnums and bignums, but
> XINT, XFASTINT, and XSETINT work only with fixnums?  I envision quite
> a few of potential bugs due to this semantic discrepancy, like the one
> you just fixed:
>
>>  - fix NUMBERP to remove redundant BIGNUMP test (ensured by INTEGERP)

Indeed. NUMBERP could include rationals if emacs lisp ever acquires a full
scheme-like numeric tower.

> Should we have a different name for what is now INTEGERP?  Like
> WHOLENUMP, for example?

INTEGERP seems natural enough, and WHOLENUMP would perhaps be confused
with NATNUMP and FIXNATP.

If anything, perhaps XINT, XFASTINT and XSETINT should be XFIXNUM,
XFASTFIXNUM and XSETFIXNUM.

    AndyM