fix f64 indexing, introduce bespoke F64Table type (#3065)

src/arithmetic.rs

@@ -138,7 +138,6 @@ impl<'a> Iterator for ArithInstructionIterator<'a> {
 #[derive(Debug)]
 pub(crate) struct ArithmeticEvaluator<'a> {
     marker: &'a mut DebrayAllocator,
-    f64_tbl: &'a F64Table,
     interm: Vec<ArithmeticTerm>,
     interm_c: usize,
 }
@@ -157,16 +156,11 @@ impl<'a> ArithmeticTermIter<'a> for &'a Term {
     }
 }
 
-fn push_literal(
+fn push_literal(interm: &mut Vec<ArithmeticTerm>, c: &Literal) -> Result<(), ArithmeticError> {
-    f64_tbl: &F64Table,
-    interm: &mut Vec<ArithmeticTerm>,
-    c: &Literal,
-) -> Result<(), ArithmeticError> {
     match c {
         Literal::Fixnum(n) => interm.push(ArithmeticTerm::Number(Number::Fixnum(*n))),
         Literal::Integer(n) => interm.push(ArithmeticTerm::Number(Number::Integer(*n))),
-        &Literal::F64Offset(offset) => {
+        &Literal::F64(_offset, n) => {
-            let n = f64_tbl.get_entry(offset);
             interm.push(ArithmeticTerm::Number(Number::Float(n)));
         }
         Literal::Rational(n) => interm.push(ArithmeticTerm::Number(Number::Rational(*n))),
@@ -186,14 +180,9 @@ fn push_literal(
 }
 
 impl<'a> ArithmeticEvaluator<'a> {
-    pub(crate) fn new(
+    pub(crate) fn new(marker: &'a mut DebrayAllocator, target_int: usize) -> Self {
-        marker: &'a mut DebrayAllocator,
-        f64_tbl: &'a F64Table,
-        target_int: usize,
-    ) -> Self {
         ArithmeticEvaluator {
             marker,
-            f64_tbl,
             interm: Vec::new(),
             interm_c: target_int,
         }
@@ -331,7 +320,7 @@ impl<'a> ArithmeticEvaluator<'a> {
 
         for term_ref in src.iter()? {
             match term_ref? {
-                ArithTermRef::Literal(c) => push_literal(self.f64_tbl, &mut self.interm, &c)?,
+                ArithTermRef::Literal(c) => push_literal(&mut self.interm, &c)?,
                 ArithTermRef::Var(lvl, cell, name) => {
                     let var_num = name.to_var_num().unwrap();
 

src/codegen.rs

@@ -6,7 +6,6 @@ use crate::forms::*;
 use crate::indexing::*;
 use crate::instructions::*;
 use crate::iterators::*;
-use crate::offset_table::F64Table;
 use crate::parser::ast::*;
 use crate::targets::*;
 use crate::types::*;
@@ -270,10 +269,9 @@ impl CodeGenSettings {
 }
 
 #[derive(Debug)]
-pub(crate) struct CodeGenerator<'f64_tbl> {
+pub(crate) struct CodeGenerator {
     marker: DebrayAllocator,
     settings: CodeGenSettings,
-    f64_tbl: &'f64_tbl F64Table,
     pub(crate) skeleton: PredicateSkeleton,
 }
 
@@ -325,7 +323,7 @@ trait AddToFreeList<'a, Target: CompilationTarget<'a>> {
     fn add_subterm_to_free_list(&mut self, term: &Term);
 }
 
-impl<'a> AddToFreeList<'a, FactInstruction> for CodeGenerator<'_> {
+impl<'a> AddToFreeList<'a, FactInstruction> for CodeGenerator {
     fn add_term_to_free_list(&mut self, r: RegType) {
         self.marker.add_reg_to_free_list(r);
     }
@@ -333,7 +331,7 @@ impl<'a> AddToFreeList<'a, FactInstruction> for CodeGenerator<'_> {
     fn add_subterm_to_free_list(&mut self, _term: &Term) {}
 }
 
-impl<'a> AddToFreeList<'a, QueryInstruction> for CodeGenerator<'_> {
+impl<'a> AddToFreeList<'a, QueryInstruction> for CodeGenerator {
     #[inline(always)]
     fn add_term_to_free_list(&mut self, _r: RegType) {}
 
@@ -355,12 +353,11 @@ fn structure_cell(term: &Term) -> Option<&Cell<RegType>> {
     }
 }
 
-impl<'f64_tbl> CodeGenerator<'f64_tbl> {
+impl CodeGenerator {
-    pub(crate) fn new(f64_tbl: &'f64_tbl F64Table, settings: CodeGenSettings) -> Self {
+    pub(crate) fn new(settings: CodeGenSettings) -> Self {
         CodeGenerator {
             marker: DebrayAllocator::new(),
             settings,
-            f64_tbl,
             skeleton: PredicateSkeleton::new(),
         }
     }
@@ -430,7 +427,7 @@ impl<'f64_tbl> CodeGenerator<'f64_tbl> {
     where
         Target: crate::targets::CompilationTarget<'a>,
         Iter: Iterator<Item = TermRef<'a>>,
-        CodeGenerator<'f64_tbl>: AddToFreeList<'a, Target>,
+        CodeGenerator: AddToFreeList<'a, Target>,
     {
         let mut target = CodeDeque::new();
 
@@ -669,7 +666,7 @@ impl<'f64_tbl> CodeGenerator<'f64_tbl> {
                 }
             },
             InlinedClauseType::IsFloat(..) => match terms[0] {
-                Term::Literal(_, Literal::F64Offset(_)) => {
+                Term::Literal(_, Literal::F64(..)) => {
                     instr!("$succeed")
                 }
                 Term::Var(ref vr, ref name) => {
@@ -690,7 +687,7 @@ impl<'f64_tbl> CodeGenerator<'f64_tbl> {
                 }
             },
             InlinedClauseType::IsNumber(..) => match terms[0] {
-                Term::Literal(_, Literal::F64Offset(_))
+                Term::Literal(_, Literal::F64(..))
                 | Term::Literal(_, Literal::Rational(_))
                 | Term::Literal(_, Literal::Integer(_))
                 | Term::Literal(_, Literal::Fixnum(_)) => {
@@ -794,7 +791,7 @@ impl<'f64_tbl> CodeGenerator<'f64_tbl> {
         term_loc: GenContext,
         arg: usize,
     ) -> Result<ArithCont, ArithmeticError> {
-        let mut evaluator = ArithmeticEvaluator::new(&mut self.marker, self.f64_tbl, target_int);
+        let mut evaluator = ArithmeticEvaluator::new(&mut self.marker, target_int);
         evaluator.compile_is(term, term_loc, arg)
     }
 
@@ -864,7 +861,7 @@ impl<'f64_tbl> CodeGenerator<'f64_tbl> {
             Term::Literal(
                 _,
                 c @ Literal::Integer(_)
-                | c @ Literal::F64Offset(_)
+                | c @ Literal::F64(..)
                 | c @ Literal::Rational(_)
                 | c @ Literal::Fixnum(_),
             ) => {

src/forms.rs

@@ -6,7 +6,6 @@ use crate::machine::disjuncts::VarData;
 use crate::machine::loader::PredicateQueue;
 use crate::machine::machine_errors::*;
 use crate::machine::machine_indices::*;
-use crate::offset_table::OffsetTable;
 use crate::parser::ast::*;
 use crate::parser::dashu::{Integer, Rational};
 use crate::parser::parser::CompositeOpDesc;

src/machine/compile.rs

@@ -1233,9 +1233,7 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> {
         let mut preprocessor = Preprocessor::new(settings);
         let clause = preprocessor.try_term_to_tl(self, term)?;
 
-        let f64_tbl = &LS::machine_st(&mut self.payload).arena.f64_tbl;
+        let mut cg = CodeGenerator::new(settings);
-
-        let mut cg = CodeGenerator::new(f64_tbl, settings);
         let clause_code = cg.compile_predicate(vec![clause])?;
 
         Ok(StandaloneCompileResult {
@@ -1264,9 +1262,7 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> {
             clauses.push(preprocessor.try_term_to_tl(self, term)?);
         }
 
-        let f64_tbl = &LS::machine_st(&mut self.payload).arena.f64_tbl;
+        let mut cg = CodeGenerator::new(settings);
-
-        let mut cg = CodeGenerator::new(f64_tbl, settings);
         let mut code = cg.compile_predicate(clauses)?;
 
         if settings.is_extensible {

src/machine/loader.rs

@@ -1413,7 +1413,7 @@ impl MachineState {
                     }
                 }
                 (HeapCellValueTag::Cons | HeapCellValueTag::Fixnum | HeapCellValueTag::F64Offset) => {
-                    term_stack.push(Term::Literal(Cell::default(), Literal::try_from(addr).unwrap()));
+                    term_stack.push(Term::Literal(Cell::default(), Literal::try_from((addr, &self.arena.f64_tbl)).unwrap()));
                 }
                 (HeapCellValueTag::StackVar, h) => {
                     term_stack.push(Term::Var(Cell::default(), VarPtr::from(format!("s_{h}"))));

src/machine/system_calls.rs

@@ -25,7 +25,6 @@ use crate::machine::partial_string::*;
 use crate::machine::stack::*;
 use crate::machine::streams::*;
 use crate::machine::{get_structure_index, Machine, VERIFY_ATTR_INTERRUPT_LOC};
-use crate::offset_table::*;
 use crate::parser::ast::*;
 use crate::parser::char_reader::*;
 use crate::parser::dashu::Integer;
@@ -1010,8 +1009,8 @@ impl MachineState {
                         Ok(Term::Literal(_, Literal::Rational(n))) => {
                             self.unify_rational(n, nx);
                         }
-                        Ok(Term::Literal(_, Literal::F64Offset(n))) => {
+                        Ok(Term::Literal(_, Literal::F64(offset, _n))) => {
-                            self.unify_f64(n, nx);
+                            self.unify_f64(offset, nx);
                         }
                         Ok(Term::Literal(_, Literal::Integer(n))) => {
                             self.unify_big_int(n, nx);

src/machine/system_calls/special_math.rs

@@ -1,5 +1,4 @@
 use crate::machine::Number;
-use crate::offset_table::OffsetTable;
 use crate::Machine;
 use ordered_float::OrderedFloat;
 use puruspe::beta::*;

src/offset_table.rs

@@ -6,7 +6,9 @@ use arcu::atomic::Arcu;
 use arcu::epoch_counters::GlobalEpochCounterPool;
 use arcu::rcu_ref::RcuRef;
 use arcu::Rcu;
-use parking_lot::RwLock;
+use fxhash::FxBuildHasher;
+use indexmap::IndexMap;
+use parking_lot::{Mutex, RwLock};
 
 use crate::machine::machine_indices::IndexPtr;
 use crate::raw_block::RawBlock;
@@ -64,25 +66,8 @@ impl<T: fmt::Debug + RawBlockTraits> OffsetTableImpl<T> {
     pub fn single_to_concurrent(&mut self) -> Arc<ConcurrentOffsetTable<T>> {
         match &mut self.0 {
             InnerOffsetTableImpl::Serial(serial_tbl) => {
-                let empty_serial_tbl = SerialOffsetTable {
+                let concurrent_tbl = Arc::new(serial_tbl.to_concurrent());
-                    block: RawBlock::empty_block(),
-                };
-
-                let serial_tbl = mem::replace(serial_tbl, empty_serial_tbl);
-                let num_tbl_entries = serial_tbl.block.size() / size_of::<T>();
-                let block = Arcu::new(serial_tbl.block, GlobalEpochCounterPool);
-
-                let offset_locks: Vec<RwLock<()>> =
-                    (0..num_tbl_entries).map(|_| RwLock::new(())).collect();
-
-                let concurrent_tbl = Arc::new(ConcurrentOffsetTable {
-                    block,
-                    growth_lock: RwLock::new(()),
-                    offset_locks: RwLock::new(offset_locks),
-                });
-
                 self.0 = InnerOffsetTableImpl::Concurrent(concurrent_tbl.clone());
-
                 concurrent_tbl
             }
             InnerOffsetTableImpl::Concurrent(concurrent_tbl) => concurrent_tbl.clone(),
@@ -94,14 +79,8 @@ impl<T: fmt::Debug + RawBlockTraits> OffsetTableImpl<T> {
         match &mut self.0 {
             InnerOffsetTableImpl::Serial(_serial_tbl) => Ok(()),
             InnerOffsetTableImpl::Concurrent(concurrent_tbl) => {
-                let table_arc = std::mem::replace(
+                let table_arc =
-                    concurrent_tbl,
+                    std::mem::replace(concurrent_tbl, Arc::new(ConcurrentOffsetTable::default()));
-                    Arc::new(ConcurrentOffsetTable {
-                        block: Arcu::new(RawBlock::empty_block(), GlobalEpochCounterPool),
-                        growth_lock: RwLock::new(()),
-                        offset_locks: RwLock::new(vec![]),
-                    }),
-                );
 
                 match Arc::try_unwrap(table_arc) {
                     Ok(table) => {
@@ -153,6 +132,9 @@ pub struct ConcurrentOffsetTable<T: RawBlockTraits> {
     offset_locks: RwLock<Vec<RwLock<()>>>,
 }
 
+unsafe impl<T: RawBlockTraits> Send for ConcurrentOffsetTable<T> {}
+unsafe impl<T: RawBlockTraits> Sync for ConcurrentOffsetTable<T> {}
+
 #[derive(Debug)]
 enum InnerOffsetTableImpl<T: RawBlockTraits> {
     Serial(SerialOffsetTable<T>),
@@ -195,28 +177,6 @@ pub trait OffsetTable<T: RawBlockTraits> {
     fn with_entry_mut<R, F: FnOnce(&mut T) -> R>(&mut self, offset: Self::Offset, f: F) -> R;
 }
 
-impl OffsetTable<OrderedFloat<f64>> for OffsetTableImpl<OrderedFloat<f64>> {
-    type Offset = F64Offset;
-
-    fn build_with(&mut self, value: OrderedFloat<f64>) -> F64Offset {
-        F64Offset(self.0.build_with(value))
-    }
-
-    #[inline]
-    fn with_entry<R, F: FnOnce(&OrderedFloat<f64>) -> R>(&self, offset: F64Offset, f: F) -> R {
-        self.0.with_entry(offset.into(), f)
-    }
-
-    #[inline]
-    fn with_entry_mut<R, F: FnOnce(&mut OrderedFloat<f64>) -> R>(
-        &mut self,
-        offset: F64Offset,
-        f: F,
-    ) -> R {
-        self.0.with_entry_mut(offset.into(), f)
-    }
-}
-
 impl OffsetTable<IndexPtr> for OffsetTableImpl<IndexPtr> {
     type Offset = CodeIndexOffset;
 
@@ -274,6 +234,27 @@ impl<T: RawBlockTraits> SerialOffsetTable<T> {
     unsafe fn lookup_mut(&mut self, offset: usize) -> &mut T {
         &mut *self.block.base.add(offset).cast::<T>().cast_mut()
     }
+
+    fn to_concurrent(&mut self) -> ConcurrentOffsetTable<T>
+    where
+        T: fmt::Debug,
+    {
+        let empty_serial_tbl = SerialOffsetTable {
+            block: RawBlock::empty_block(),
+        };
+
+        let serial_tbl = mem::replace(self, empty_serial_tbl);
+        let num_tbl_entries = serial_tbl.block.size() / size_of::<T>();
+        let block = Arcu::new(serial_tbl.block, GlobalEpochCounterPool);
+
+        let offset_locks: Vec<RwLock<()>> = (0..num_tbl_entries).map(|_| RwLock::new(())).collect();
+
+        ConcurrentOffsetTable {
+            block,
+            growth_lock: RwLock::new(()),
+            offset_locks: RwLock::new(offset_locks),
+        }
+    }
 }
 
 impl<T: RawBlockTraits> ConcurrentOffsetTable<T> {
@@ -359,7 +340,145 @@ impl<T: RawBlockTraits> ConcurrentOffsetTable<T> {
     }
 }
 
-pub type F64Table = OffsetTableImpl<OrderedFloat<f64>>;
+impl<T: fmt::Debug + RawBlockTraits> Default for ConcurrentOffsetTable<T> {
+    fn default() -> ConcurrentOffsetTable<T> {
+        Self {
+            block: Arcu::new(RawBlock::empty_block(), GlobalEpochCounterPool),
+            growth_lock: RwLock::new(()),
+            offset_locks: RwLock::new(vec![]),
+        }
+    }
+}
+
+/*
+ * indirection_tbl maps f64 values to unique offsets so predicate indices on floats work correctly.
+ */
+
+#[derive(Debug)]
+pub struct ConcurrentF64Table {
+    indirection_tbl: Mutex<IndexMap<OrderedFloat<f64>, F64Offset, FxBuildHasher>>,
+    offset_tbl: ConcurrentOffsetTable<OrderedFloat<f64>>,
+}
+
+#[derive(Debug)]
+pub struct SerialF64Table {
+    indirection_tbl: IndexMap<OrderedFloat<f64>, F64Offset, FxBuildHasher>,
+    offset_tbl: SerialOffsetTable<OrderedFloat<f64>>,
+}
+
+#[derive(Debug)]
+pub enum F64Table {
+    Serial(SerialF64Table),
+    #[allow(dead_code)]
+    Concurrent(Arc<ConcurrentF64Table>),
+}
+
+impl F64Table {
+    pub fn new() -> Self {
+        Self::Serial(SerialF64Table {
+            indirection_tbl: IndexMap::with_hasher(FxBuildHasher::new()),
+            offset_tbl: SerialOffsetTable::new(),
+        })
+    }
+
+    pub fn build_with(&mut self, value: OrderedFloat<f64>) -> F64Offset {
+        match self {
+            F64Table::Serial(serial_tbl) => {
+                if let Some(offset) = serial_tbl.indirection_tbl.get(&value).cloned() {
+                    return offset;
+                }
+
+                let offset = F64Offset(unsafe { serial_tbl.offset_tbl.build_with(value) });
+                serial_tbl.indirection_tbl.insert(value, offset);
+
+                offset
+            }
+            F64Table::Concurrent(concurrent_tbl) => {
+                {
+                    let indirection_tbl = concurrent_tbl.indirection_tbl.lock();
+
+                    if let Some(offset) = indirection_tbl.get(&value).cloned() {
+                        return offset;
+                    }
+                }
+
+                let offset = F64Offset(concurrent_tbl.offset_tbl.build_with(value));
+                concurrent_tbl.indirection_tbl.lock().insert(value, offset);
+                offset
+            }
+        }
+    }
+
+    #[inline]
+    pub fn get_entry(&self, offset: F64Offset) -> OrderedFloat<f64> {
+        match self {
+            F64Table::Serial(serial_tbl) => unsafe { *serial_tbl.offset_tbl.lookup(offset.into()) },
+            F64Table::Concurrent(concurrent_tbl) => concurrent_tbl
+                .offset_tbl
+                .with_entry(offset.into(), |value| *value),
+        }
+    }
+
+    #[must_use = "the returned concurrent table must be absorbed into the owned F64Table"]
+    pub fn single_to_concurrent(&mut self) -> Arc<ConcurrentF64Table> {
+        match self {
+            F64Table::Serial(serial_tbl) => {
+                let offset_tbl = serial_tbl.offset_tbl.to_concurrent();
+
+                Arc::new(ConcurrentF64Table {
+                    indirection_tbl: Mutex::new(mem::replace(
+                        &mut serial_tbl.indirection_tbl,
+                        IndexMap::with_hasher(FxBuildHasher::new()),
+                    )),
+                    offset_tbl,
+                })
+            }
+            F64Table::Concurrent(concurrent_tbl) => concurrent_tbl.clone(),
+        }
+    }
+
+    #[must_use = "the transition to a single-threaded offset table may fail if the concurrent table is held from multiple places"]
+    pub fn concurrent_to_single(&mut self) -> Result<(), ()> {
+        match self {
+            F64Table::Serial { .. } => Ok(()),
+            F64Table::Concurrent(concurrent_f64_tbl) => {
+                let table_arc = std::mem::replace(
+                    concurrent_f64_tbl,
+                    Arc::new(ConcurrentF64Table {
+                        indirection_tbl: Mutex::new(IndexMap::with_hasher(FxBuildHasher::new())),
+                        offset_tbl: ConcurrentOffsetTable::default(),
+                    }),
+                );
+
+                match Arc::try_unwrap(table_arc) {
+                    Ok(ConcurrentF64Table {
+                        indirection_tbl,
+                        offset_tbl,
+                    }) => {
+                        // this was the only instance of the concurrent table, as such
+                        // at this point no build_with/with_entry{_mut} call can be in-progress/made
+
+                        // this shouldn't be able to fail
+                        let raw_block =
+                            Arc::try_unwrap(offset_tbl.block.replace(RawBlock::empty_block()))
+                                .unwrap();
+                        *self = Self::Serial(SerialF64Table {
+                            indirection_tbl: indirection_tbl.into_inner(),
+                            offset_tbl: SerialOffsetTable { block: raw_block },
+                        });
+
+                        Ok(())
+                    }
+                    Err(table_arc) => {
+                        *concurrent_f64_tbl = table_arc;
+                        Err(())
+                    }
+                }
+            }
+        }
+    }
+}
+
 pub type CodeIndexTable = OffsetTableImpl<IndexPtr>;
 
 #[derive(Clone, Copy, Debug)]

src/parser/ast.rs

@@ -22,6 +22,7 @@ use dashu::Integer;
 use dashu::Rational;
 use fxhash::FxBuildHasher;
 use indexmap::IndexMap;
+use ordered_float::OrderedFloat;
 use scryer_modular_bitfield::error::OutOfBounds;
 use scryer_modular_bitfield::prelude::*;
 
@@ -706,7 +707,7 @@ pub enum Literal {
     Fixnum(Fixnum),
     Integer(TypedArenaPtr<Integer>),
     Rational(TypedArenaPtr<Rational>),
-    F64Offset(F64Offset),
+    F64(F64Offset, OrderedFloat<f64>),
 }
 
 /*

src/parser/lexer.rs

@@ -54,28 +54,10 @@ impl Token {
     }
 }
 
-#[derive(Debug)]
-enum Number {
-    BigInt(TypedArenaPtr<Integer>),
-    Fixnum(Fixnum),
-    Float(F64Offset),
-}
-
-impl Number {
-    #[inline]
-    #[allow(clippy::wrong_self_convention)]
-    fn to_literal(self) -> Literal {
-        match self {
-            Number::BigInt(ibig) => Literal::Integer(ibig),
-            Number::Fixnum(fixnum) => Literal::Fixnum(fixnum),
-            Number::Float(f) => Literal::F64Offset(f),
-        }
-    }
-}
-
 #[derive(Debug)]
 enum NumberToken {
-    Number(Number),
+    Integer(GInteger),
+    Float(F64Offset, OrderedFloat<f64>),
     Partial(String),
 }
 
@@ -84,7 +66,9 @@ impl NumberToken {
     #[allow(clippy::wrong_self_convention)]
     fn to_token(self) -> Option<Token> {
         match self {
-            NumberToken::Number(number) => Some(Token::Literal(number.to_literal())),
+            NumberToken::Float(offset, fl) => Some(Token::Literal(Literal::F64(offset, fl))),
+            NumberToken::Integer(GInteger::BigInt(n)) => Some(Token::Literal(Literal::Integer(n))),
+            NumberToken::Integer(GInteger::Fixnum(n)) => Some(Token::Literal(Literal::Fixnum(n))),
             NumberToken::Partial(_) => None,
         }
     }
@@ -105,6 +89,22 @@ macro_rules! try_nt {
     }};
 }
 
+#[derive(Debug)]
+enum GInteger {
+    BigInt(TypedArenaPtr<Integer>),
+    Fixnum(Fixnum),
+}
+
+impl GInteger {
+    #[inline]
+    fn to_literal(self) -> Literal {
+        match self {
+            GInteger::BigInt(integer) => Literal::Integer(integer),
+            GInteger::Fixnum(fixnum) => Literal::Fixnum(fixnum),
+        }
+    }
+}
+
 pub(crate) struct Lexer<'a, R> {
     pub(crate) reader: R,
     pub(crate) machine_st: &'a mut MachineState,
@@ -523,7 +523,7 @@ impl<'a, R: CharRead> Lexer<'a, R> {
             }
 
             self.parse_integer_by_radix(&token, 16)
-                .map(NumberToken::Number)
+                .map(NumberToken::Integer)
         } else {
             self.return_char(start);
             Err(ParserError::ParseBigInt(self.line_num, self.col_num))
@@ -554,7 +554,7 @@ impl<'a, R: CharRead> Lexer<'a, R> {
             }
 
             self.parse_integer_by_radix(&token, 8)
-                .map(NumberToken::Number)
+                .map(NumberToken::Integer)
         } else {
             self.return_char(start);
             Err(ParserError::ParseBigInt(self.line_num, self.col_num))
@@ -585,7 +585,7 @@ impl<'a, R: CharRead> Lexer<'a, R> {
             }
 
             self.parse_integer_by_radix(&token, 2)
-                .map(NumberToken::Number)
+                .map(NumberToken::Integer)
         } else {
             self.return_char(start);
             Err(ParserError::ParseBigInt(self.line_num, self.col_num))
@@ -680,10 +680,16 @@ impl<'a, R: CharRead> Lexer<'a, R> {
         }
     }
 
-    fn vacate_with_float(&mut self, mut token: String) -> Result<Number, ParserError> {
+    fn vacate_with_float(
+        &mut self,
+        mut token: String,
+    ) -> Result<(F64Offset, OrderedFloat<f64>), ParserError> {
         self.return_char(token.pop().unwrap());
+
         let n = parse_float_lossy(&token)?;
-        Ok(Number::Float(float_alloc!(n, self.machine_st.arena)))
+        let offset = float_alloc!(n, self.machine_st.arena);
+
+        Ok((offset, OrderedFloat(n)))
     }
 
     fn skip_underscore_in_number(&mut self) -> Result<char, ParserError> {
@@ -704,24 +710,24 @@ impl<'a, R: CharRead> Lexer<'a, R> {
         }
     }
 
-    fn parse_integer_by_radix(&mut self, token: &str, radix: u32) -> Result<Number, ParserError> {
+    fn parse_integer_by_radix(&mut self, token: &str, radix: u32) -> Result<GInteger, ParserError> {
         i64::from_str_radix(token, radix)
             .map(|n| {
                 Fixnum::build_with_checked(n)
-                    .map(Number::Fixnum)
+                    .map(GInteger::Fixnum)
                     .unwrap_or_else(|_| {
-                        Number::BigInt(arena_alloc!(Integer::from(n), &mut self.machine_st.arena))
+                        GInteger::BigInt(arena_alloc!(Integer::from(n), &mut self.machine_st.arena))
                     })
             })
             .or_else(|_| {
                 Integer::from_str_radix(token, radix)
-                    .map(|n| Number::BigInt(arena_alloc!(n, &mut self.machine_st.arena)))
+                    .map(|n| GInteger::BigInt(arena_alloc!(n, &mut self.machine_st.arena)))
                     .map_err(|_| ParserError::ParseBigInt(self.line_num, self.col_num))
             })
     }
 
     #[inline]
-    fn parse_integer(&mut self, token: &str) -> Result<Number, ParserError> {
+    fn parse_integer(&mut self, token: &str) -> Result<GInteger, ParserError> {
         self.parse_integer_by_radix(token, 10)
     }
 
@@ -743,7 +749,7 @@ impl<'a, R: CharRead> Lexer<'a, R> {
 
             if self.reader.peek_char().is_none() {
                 self.return_char('.');
-                self.parse_integer(&token).map(NumberToken::Number)
+                self.parse_integer(&token).map(NumberToken::Integer)
             } else if decimal_digit_char!(self.lookahead_char()?) {
                 token.push('.');
                 token.push(self.read_char()?);
@@ -761,12 +767,18 @@ impl<'a, R: CharRead> Lexer<'a, R> {
                     token.push(c);
 
                     let c = match self.lookahead_char() {
-                        Err(_) => return self.vacate_with_float(token).map(NumberToken::Number),
+                        Err(_) => {
+                            return self
+                                .vacate_with_float(token)
+                                .map(|(offset, fl)| NumberToken::Float(offset, fl))
+                        }
                         Ok(c) => c,
                     };
 
                     if !sign_char!(c) && !decimal_digit_char!(c) {
-                        return self.vacate_with_float(token).map(NumberToken::Number);
+                        return self
+                            .vacate_with_float(token)
+                            .map(|(offset, fl)| NumberToken::Float(offset, fl));
                     }
 
                     if sign_char!(c) {
@@ -776,14 +788,18 @@ impl<'a, R: CharRead> Lexer<'a, R> {
                         let c = match self.lookahead_char() {
                             Err(_) => {
                                 self.return_char(token.pop().unwrap());
-                                return self.vacate_with_float(token).map(NumberToken::Number);
+                                return self
+                                    .vacate_with_float(token)
+                                    .map(|(offset, fl)| NumberToken::Float(offset, fl));
                             }
                             Ok(c) => c,
                         };
 
                         if !decimal_digit_char!(c) {
                             self.return_char(token.pop().unwrap());
-                            return self.vacate_with_float(token).map(NumberToken::Number);
+                            return self
+                                .vacate_with_float(token)
+                                .map(|(offset, fl)| NumberToken::Float(offset, fl));
                         }
                     }
 
@@ -805,30 +821,27 @@ impl<'a, R: CharRead> Lexer<'a, R> {
                         }
 
                         let n = parse_float_lossy(&token)?;
+                        let offset = float_alloc!(n, self.machine_st.arena);
 
-                        Ok(NumberToken::Number(Number::Float(float_alloc!(
+                        Ok(NumberToken::Float(offset, OrderedFloat(n)))
-                            n,
-                            self.machine_st.arena
-                        ))))
                     } else {
-                        return self.vacate_with_float(token).map(NumberToken::Number);
+                        self.vacate_with_float(token)
+                            .map(|(offset, fl)| NumberToken::Float(offset, fl))
                     }
                 } else {
                     let n = parse_float_lossy(&token)?;
-                    Ok(NumberToken::Number(Number::Float(float_alloc!(
+                    let offset = float_alloc!(n, self.machine_st.arena);
-                        n,
+                    Ok(NumberToken::Float(offset, OrderedFloat(n)))
-                        self.machine_st.arena
-                    ))))
                 }
             } else {
                 self.return_char('.');
-                self.parse_integer(&token).map(NumberToken::Number)
+                self.parse_integer(&token).map(NumberToken::Integer)
             }
         } else if token.starts_with('0') && token.len() == 1 {
             if c == 'x' {
                 self.hexadecimal_constant(c).or_else(|e| {
                     if let ParserError::ParseBigInt(..) = e {
-                        self.parse_integer(&token).map(NumberToken::Number)
+                        self.parse_integer(&token).map(NumberToken::Integer)
                     } else {
                         Err(e)
                     }
@@ -836,7 +849,7 @@ impl<'a, R: CharRead> Lexer<'a, R> {
             } else if c == 'o' {
                 self.octal_constant(c).or_else(|e| {
                     if let ParserError::ParseBigInt(..) = e {
-                        self.parse_integer(&token).map(NumberToken::Number)
+                        self.parse_integer(&token).map(NumberToken::Integer)
                     } else {
                         Err(e)
                     }
@@ -844,7 +857,7 @@ impl<'a, R: CharRead> Lexer<'a, R> {
             } else if c == 'b' {
                 self.binary_constant(c).or_else(|e| {
                     if let ParserError::ParseBigInt(..) = e {
-                        self.parse_integer(&token).map(NumberToken::Number)
+                        self.parse_integer(&token).map(NumberToken::Integer)
                     } else {
                         Err(e)
                     }
@@ -861,14 +874,16 @@ impl<'a, R: CharRead> Lexer<'a, R> {
                         self.skip_char(c);
                         self.return_char('\'');
 
-                        return Ok(NumberToken::Number(Number::Fixnum(Fixnum::build_with(0))));
+                        return Ok(NumberToken::Integer(GInteger::Fixnum(Fixnum::build_with(
+                            0,
+                        ))));
                     } else {
                         self.return_char('\\');
                     }
                 }
 
                 self.get_single_quoted_char()
-                    .map(|c| NumberToken::Number(Number::Fixnum(Fixnum::build_with(c))))
+                    .map(|c| NumberToken::Integer(GInteger::Fixnum(Fixnum::build_with(c))))
                     .or_else(|err| {
                         match err {
                             ParserError::UnexpectedChar('\'', ..) => {}
@@ -876,13 +891,13 @@ impl<'a, R: CharRead> Lexer<'a, R> {
                         }
 
                         self.return_char(c);
-                        self.parse_integer(&token).map(NumberToken::Number)
+                        self.parse_integer(&token).map(NumberToken::Integer)
                     })
             } else {
-                self.parse_integer(&token).map(NumberToken::Number)
+                self.parse_integer(&token).map(NumberToken::Integer)
             }
         } else {
-            self.parse_integer(&token).map(NumberToken::Number)
+            self.parse_integer(&token).map(NumberToken::Integer)
         }
     }
 
@@ -954,13 +969,12 @@ impl<'a, R: CharRead> Lexer<'a, R> {
                 Ok(n) => Ok(Token::Literal(n.to_literal())),
                 Err(_) => {
                     let n = parse_float_lossy(&token_string)?;
-                    Ok(Token::Literal(Literal::F64Offset(float_alloc!(
+                    let offset = float_alloc!(n, self.machine_st.arena);
-                        n,
+                    Ok(Token::Literal(Literal::F64(offset, OrderedFloat(n))))
-                        self.machine_st.arena
-                    ))))
                 }
             },
-            Ok(NumberToken::Number(n)) => Ok(Token::Literal(n.to_literal())),
+            Ok(NumberToken::Float(offset, n)) => Ok(Token::Literal(Literal::F64(offset, n))),
+            Ok(NumberToken::Integer(n)) => Ok(Token::Literal(n.to_literal())),
             Err(e) => Err(e),
         }
     }

src/parser/parser.rs

@@ -1,9 +1,10 @@
 use dashu::Integer;
 use dashu::Rational;
+use ordered_float::OrderedFloat;
 
 use crate::arena::*;
 use crate::atom_table::*;
-use crate::offset_table::OffsetTable;
+use crate::offset_table::F64Offset;
 use crate::parser::ast::*;
 use crate::parser::char_reader::*;
 use crate::parser::lexer::*;
@@ -911,7 +912,7 @@ impl<'a, R: CharRead> Parser<'a, R> {
     fn negate_number<N, Negator, ToLiteral>(&mut self, n: N, negator: Negator, constr: ToLiteral)
     where
         Negator: Fn(N, &mut Arena) -> N,
-        ToLiteral: Fn(N, &mut Arena) -> Literal,
+        ToLiteral: Fn(N) -> Literal,
     {
         if let Some(desc) = self.stack.last().cloned() {
             if let Some(term) = self.terms.last().cloned() {
@@ -924,7 +925,7 @@ impl<'a, R: CharRead> Parser<'a, R> {
                         self.terms.pop();
 
                         let arena = &mut self.lexer.machine_st.arena;
-                        let literal = constr(negator(n, arena), arena);
+                        let literal = constr(negator(n, arena));
 
                         self.shift(Token::Literal(literal), 0, TERM);
 
@@ -935,7 +936,7 @@ impl<'a, R: CharRead> Parser<'a, R> {
             }
         }
 
-        let literal = constr(n, &mut self.lexer.machine_st.arena);
+        let literal = constr(n);
         self.shift(Token::Literal(literal), 0, TERM);
     }
 
@@ -952,42 +953,36 @@ impl<'a, R: CharRead> Parser<'a, R> {
             arena_alloc!(data, arena)
         }
 
+        fn negate_f64(
+            (_offset, n): (F64Offset, OrderedFloat<f64>),
+            arena: &mut Arena,
+        ) -> (F64Offset, OrderedFloat<f64>) {
+            let offset = arena.f64_tbl.build_with(-n);
+            (offset, -n)
+        }
+
         match token {
             Token::String(string) => {
                 self.shift(Token::String(string), 0, TERM);
             }
             Token::Literal(Literal::Integer(n)) => {
-                self.negate_number(n, negate_int_rc, |n, _| Literal::Integer(n))
+                self.negate_number(n, negate_int_rc, Literal::Integer)
             }
             Token::Literal(Literal::Rational(n)) => {
-                self.negate_number(n, negate_rat_rc, |r, _| Literal::Rational(r))
+                self.negate_number(n, negate_rat_rc, Literal::Rational)
             }
-            Token::Literal(Literal::F64Offset(n))
+            Token::Literal(Literal::F64(_offset, n)) if n.is_infinite() => {
-                if self
-                    .lexer
-                    .machine_st
-                    .arena
-                    .f64_tbl
-                    .get_entry(n)
-                    .is_infinite() =>
-            {
                 return Err(ParserError::InfiniteFloat(
                     self.lexer.line_num,
                     self.lexer.col_num,
                 ));
             }
-            Token::Literal(Literal::F64Offset(n)) => {
+            Token::Literal(Literal::F64(offset, n)) => {
-                let n = self.lexer.machine_st.arena.f64_tbl.get_entry(n);
+                self.negate_number((offset, n), negate_f64, |(offset, n)| {
-
+                    Literal::F64(offset, n)
-                self.negate_number(
+                })
-                    n,
-                    |n, _| -n,
-                    |n, arena| Literal::F64Offset(arena.f64_tbl.build_with(n)),
-                )
-            }
-            Token::Literal(Literal::Fixnum(n)) => {
-                self.negate_number(n, |n, _| -n, |n, _| Literal::Fixnum(n))
             }
+            Token::Literal(Literal::Fixnum(n)) => self.negate_number(n, |n, _| -n, Literal::Fixnum),
             Token::Literal(c) => {
                 if let Literal::Atom(name) = c {
                     if !self.shift_op(name, op_dir)? {

src/types.rs

@@ -313,15 +313,15 @@ impl From<Literal> for HeapCellValue {
             Literal::Rational(bigint_ptr) => {
                 typed_arena_ptr_as_cell!(bigint_ptr)
             }
-            Literal::F64Offset(f) => HeapCellValue::from(f),
+            Literal::F64(offset, _n) => HeapCellValue::from(offset),
         }
     }
 }
 
-impl TryFrom<HeapCellValue> for Literal {
+impl TryFrom<(HeapCellValue, &'_ F64Table)> for Literal {
     type Error = ();
 
-    fn try_from(value: HeapCellValue) -> Result<Literal, ()> {
+    fn try_from((value, f64_tbl): (HeapCellValue, &F64Table)) -> Result<Literal, ()> {
         read_heap_cell!(value,
             (HeapCellValueTag::Atom, (name, arity)) => {
                 if arity == 0 {
@@ -333,8 +333,8 @@ impl TryFrom<HeapCellValue> for Literal {
             (HeapCellValueTag::Fixnum, n) => {
                 Ok(Literal::Fixnum(n))
             }
-            (HeapCellValueTag::F64Offset, f) => {
+            (HeapCellValueTag::F64Offset, offset) => {
-                Ok(Literal::F64Offset(f))
+                Ok(Literal::F64(offset, f64_tbl.get_entry(offset)))
             }
             (HeapCellValueTag::CodeIndexOffset, idx) => {
                 Ok(Literal::CodeIndexOffset(idx))

tests-pl/ffi_f64_minus_zero.pl

@@ -12,7 +12,7 @@ test :-
     ffi:'signum'(A, SA),
     ffi:'signum'(B, SB),
     write((SA, SB)),
-    -1.0 is SA, % incorrect, based on https://www.swi-prolog.org/pldoc/man?function=max/2 -0.0 is less than 0.0 so A and B should be 0.0 for which signum should be 1
+    1.0 is SA,
     1.0 is SB.
 
 :- initialization(test).

tests/scryer/ffi.rs

@@ -83,7 +83,7 @@ fn ffi_f64_minus_zero() {
     load_module_test_with_input(
         "tests-pl/ffi_f64_minus_zero.pl",
         format!("LIB={dynlib_path:?}."),
-        "-1.0,1.0",
+        "1.0,1.0",
     );
 }