From d9d789fdd02455e9034e9c8cbc4070ff2971077f Mon Sep 17 00:00:00 2001 From: Simon Brooke Date: Thu, 3 Jan 2019 11:21:08 +0000 Subject: [PATCH] Now creating the correct internal bignum representation add_integers returns an integer which by inspection of the internal representation is correct, but the print representation is not correct. --- lisp/expt.lisp | 2 ++ notes/bignums.md | 4 +-- src/arith/integer.c | 58 +++++++++++++++++++++++++------------------- src/ops/read.c | 3 +++ unit-tests/bignum.sh | 14 +++++++++++ 5 files changed, 54 insertions(+), 27 deletions(-) create mode 100644 unit-tests/bignum.sh diff --git a/lisp/expt.lisp b/lisp/expt.lisp index db6a7b3..af1fff1 100644 --- a/lisp/expt.lisp +++ b/lisp/expt.lisp @@ -4,3 +4,5 @@ (cond ((= x 1) n) (t (* n (expt n (- x 1))))))) + +(expt 2 65) diff --git a/notes/bignums.md b/notes/bignums.md index ea4b0b3..f77653c 100644 --- a/notes/bignums.md +++ b/notes/bignums.md @@ -2,6 +2,6 @@ Each integer comprises at least one cell of type INTR, holding a signed 64 bit integer with a value in the range 0 ... MAX-INTEGER, where the actual value of MAX-INTEGER does not need to be the same as the C language LONG\_MAX, provided that it is less than this. It seems to me that a convenient number would be the largest number less than LONG\_MAX which has all bits set -LONG\_MAX is 0x7FFFFFFFFFFFFFFF, so the number we're looking for is 0xFFFFFFFFFFFFFFF, which is 1,152,921,504,606,846,975, which is 2^60 - 1. This means we can use bit masking with 0xFFFFFFFFFFFFFFF to extract the part of **int64_t** which will fit in a single cell. +LONG\_MAX is 0x7FFFFFFFFFFFFFFF, so the number we're looking for is 0x0FFFFFFFFFFFFFFF, which is 1,152,921,504,606,846,975, which is 2^60 - 1. This means we can use bit masking with 0xFFFFFFFFFFFFFFF to extract the part of **int64_t** which will fit in a single cell. -It also means that if we multiply two **int64_t**s into an **__int128_t**, we can then right-shift by 60 places to get the carry. \ No newline at end of file +It also means that if we multiply two **int64_t**s into an **__int128_t**, we can then right-shift by 60 places to get the carry. diff --git a/src/arith/integer.c b/src/arith/integer.c index f7bb77d..957b6bb 100644 --- a/src/arith/integer.c +++ b/src/arith/integer.c @@ -31,7 +31,7 @@ /* * The maximum value we will allow in an integer cell. */ -#define MAX_INTEGER ((__int128_t)0xFFFFFFFFFFFFFFF) +#define MAX_INTEGER ((__int128_t)0x0fffffffffffffffL) /** * hexadecimal digits for printing numbers. @@ -109,9 +109,9 @@ struct cons_pointer add_integers( struct cons_pointer a, if ( integerp( a ) && integerp( b ) ) { debug_print( L"add_integers: ", DEBUG_ARITH ); debug_print_object( a, DEBUG_ARITH ); - debug_print( L" x ", DEBUG_ARITH ); + debug_print( L" + ", DEBUG_ARITH ); debug_print_object( b, DEBUG_ARITH ); - debug_printf( DEBUG_ARITH, L"; carry = %ld\n", carry ); + debug_println( DEBUG_ARITH); while ( !nilp( a ) || !nilp( b ) || carry != 0 ) { __int128_t av = @@ -133,16 +133,20 @@ struct cons_pointer add_integers( struct cons_pointer a, rv = rv & MAX_INTEGER; } - struct cons_pointer tail = make_integer( (int64_t)(rv << 64), NIL); + struct cons_pointer tail = make_integer( (int64_t)rv, NIL); if (nilp(cursor)) { cursor = tail; } else { - inc_ref(tail); - /* yes, this is a destructive change - but the integer has not yet been released - * into the wild */ - struct cons_space_object * c = &pointer2cell(cursor); - c->payload.integer.more = tail; + inc_ref(tail); + /* yes, this is a destructive change - but the integer has not yet been released + * into the wild */ + struct cons_space_object * c = &pointer2cell(cursor); + c->payload.integer.more = tail; + } + + if ( nilp(result) ) { + result = cursor; } a = pointer2cell( a ).payload.integer.more; @@ -150,7 +154,7 @@ struct cons_pointer add_integers( struct cons_pointer a, } } debug_print( L"add_integers returning: ", DEBUG_ARITH ); - debug_print_object( result, DEBUG_ARITH ); + debug_dump_object( result, DEBUG_ARITH ); debug_println( DEBUG_ARITH ); return result; @@ -167,10 +171,10 @@ struct cons_pointer multiply_integers( struct cons_pointer a, __int128_t carry = 0; if ( integerp( a ) && integerp( b ) ) { - debug_print( L"multiply_integers: ", DEBUG_ARITH ); - debug_print_object( a, DEBUG_ARITH ); - debug_print( L" x ", DEBUG_ARITH ); - debug_print_object( b, DEBUG_ARITH ); + debug_print( L"multiply_integers: \n", DEBUG_ARITH ); + debug_dump_object( a, DEBUG_ARITH ); + debug_print( L" x \n", DEBUG_ARITH ); + debug_dump_object( b, DEBUG_ARITH ); debug_println( DEBUG_ARITH ); while ( !nilp( a ) || !nilp( b ) || carry != 0 ) { @@ -196,19 +200,19 @@ struct cons_pointer multiply_integers( struct cons_pointer a, debug_printf( DEBUG_ARITH, L"multiply_integers: 64 bit overflow; setting carry to %ld\n", (int64_t)carry ); - rv = rv & MAX_INTEGER; + rv &= MAX_INTEGER; // <<< PROBLEM IS HERE! } - struct cons_pointer tail = make_integer( (int64_t)(rv << 64), NIL); + struct cons_pointer tail = make_integer( (int64_t)rv, NIL); if (nilp(cursor)) { cursor = tail; } else { - inc_ref(tail); - /* yes, this is a destructive change - but the integer has not yet been released - * into the wild */ - struct cons_space_object * c = &pointer2cell(cursor); - c->payload.integer.more = tail; + inc_ref(tail); + /* yes, this is a destructive change - but the integer has not yet been released + * into the wild */ + struct cons_space_object * c = &pointer2cell(cursor); + c->payload.integer.more = tail; } if ( nilp(result) ) { @@ -220,8 +224,8 @@ struct cons_pointer multiply_integers( struct cons_pointer a, } } - debug_print( L"multiply_integers returning: ", DEBUG_ARITH ); - debug_print_object( result, DEBUG_ARITH ); + debug_print( L"multiply_integers returning:\n", DEBUG_ARITH ); + debug_dump_object( result, DEBUG_ARITH ); debug_println( DEBUG_ARITH ); return result; @@ -260,7 +264,11 @@ struct cons_pointer integer_to_string( struct cons_pointer int_pointer, accumulator = llabs( accumulator ); int digits = 0; - if ( accumulator == 0 ) { + if ( accumulator == 0 && !nilp(integer.payload.integer.more) ) { + accumulator = MAX_INTEGER; + } + + if ( accumulator == 0) { result = c_string_to_lisp_string( L"0" ); } else { while ( accumulator > 0 ) { @@ -291,7 +299,7 @@ struct cons_pointer integer_to_string( struct cons_pointer int_pointer, accumulator += ( base * ( i / base ) ); } } - + if (stringp(result) && pointer2cell(result).payload.string.character == L',') { /* if the number of digits in the string is divisible by 3, there will be * an unwanted comma on the front. */ diff --git a/src/ops/read.c b/src/ops/read.c index c83fc24..cc035a1 100644 --- a/src/ops/read.c +++ b/src/ops/read.c @@ -161,6 +161,9 @@ struct cons_pointer read_number( struct stack_frame *frame, wint_t initial, bool seen_period ) { debug_print( L"entering read_number\n", DEBUG_IO ); struct cons_pointer result = NIL; + + /* TODO: accumulator and dividend cannot be `int64_t`s, otherwise we cannot + * read bignums. They will have to be Lisp integers. */ int64_t accumulator = 0; int64_t dividend = 0; int places_of_decimals = 0; diff --git a/unit-tests/bignum.sh b/unit-tests/bignum.sh new file mode 100644 index 0000000..aa29143 --- /dev/null +++ b/unit-tests/bignum.sh @@ -0,0 +1,14 @@ +#!/bin/bash + +expected='1,152,921,504,606,846,976' +# 1,152,921,504,606,846,975 is the largest single cell positive integer; +# consequently 1,152,921,504,606,846,976 is the first two cell positive integer. +actual=`echo '(+ 1,152,921,504,606,846,975 1)' | target/psse -v 68 2>bignum.log | tail -1` + +if [ "${expected}" = "${actual}" ] +then + echo "OK" +else + echo "Fail: expected '${expected}', got '${actual}'" + exit 1 +fi