Fix up some stuff around closures. Reinforces the point that I need a…

… better system for memory representation on heap

.vscode/launch.json

@@ -11,17 +11,17 @@
             "cargo": {
                 "args": [
                     "build",
-                    "--release",
+                    // "--release",
                     "--bin=main",
                     "--package=main",
-                    "--features=compacting"
+                    "--features=simple-mark-and-sweep,thread-safe"
                 ],
                 "filter": {
                     "name": "main",
                     "kind": "bin"
                 }
             },
-            "args": ["resources/grab_bag.bg"],
+            "args": ["resources/thread.bg"],
             "cwd": "${workspaceFolder}"
         },
         {

resources/thread.bg

@@ -65,23 +65,21 @@ fn call_function() {
 
 fn swap_counter_in_many_threads(counter, n) {
     let f = fn() {
-        let y = counter;
+        swap!(counter, fn(x) { x + 1 })
     };
 
-    // TODO: This errors out because n is no longer a number
-    // it is somehow counter.
-    // It seems to have something to do with the closure capturing
-    // the counter variable
-    println(n)
-    println(n - 1)
+    if n > 0 {
+        thread(f);
+        swap_counter_in_many_threads(counter, n - 1)
+    }
 }
 
 // You can see that thing get's replaced by one of these nodes
 // This is because the gc is not working correctly with concurrency
 fn main() {
 
     let counter = atom(0);
-    // swap_counter_in_many_threads(counter, 10)
+    swap_counter_in_many_threads(counter, 10)
     call_function()
     swap!(counter, fn(x) { x + 1 })
     call_function()
@@ -120,4 +118,7 @@ fn main() {
     "done"
 }
 
-// TODO: This breaks with compacting gc
+// thread-safe
+// Expect
+// 22
+// done

src/arm.rs

@@ -576,11 +576,16 @@ impl LowLevelArm {
         });
     }
 
-    pub fn pop_from_stack(&mut self, reg: Register, offset: i32) {
+    pub fn pop_from_stack_indexed(&mut self, reg: Register, offset: i32) {
         self.increment_stack_size(-1);
         self.load_from_stack(reg, -(offset + self.max_locals + 1))
     }
 
+    pub fn pop_from_stack(&mut self, reg: Register) {
+        self.increment_stack_size(-1);
+        self.load_from_stack(reg, -(self.max_locals + self.stack_size + 1))
+    }
+
     pub fn load_local(&mut self, destination: Register, offset: i32) {
         self.load_from_stack(destination, -(offset + 1));
     }

src/ast.rs

@@ -631,13 +631,13 @@ impl<'a> AstCompiler<'a> {
         //     ree_variables: *const Value,
         // }
         let closure_register = self.ir.untag(closure_register.into());
-        let function_pointer = self.ir.load_from_memory(closure_register, 0);
+        let function_pointer = self.ir.load_from_memory(closure_register, 1);
 
         self.ir.assign(function_register, function_pointer);
 
         // TODO: I need to fix how these are stored on the stack
 
-        let num_free_variables = self.ir.load_from_memory(closure_register, 1);
+        let num_free_variables = self.ir.load_from_memory(closure_register, 2);
         let num_free_variables = self.ir.tag(num_free_variables, BuiltInTypes::Int.get_tag());
         // for each variable I need to push them onto the stack after the prelude
         let loop_start = self.ir.label("loop_start");
@@ -651,7 +651,7 @@ impl<'a> AstCompiler<'a> {
             counter,
             num_free_variables,
         );
-        let free_variable_offset = self.ir.add(counter, Value::SignedConstant(3));
+        let free_variable_offset = self.ir.add(counter, Value::SignedConstant(4));
         let free_variable_offset = self.ir.mul(free_variable_offset, Value::SignedConstant(8));
         // TODO: This needs to change based on counter
         let free_variable_offset = self.ir.untag(free_variable_offset);
@@ -660,7 +660,7 @@ impl<'a> AstCompiler<'a> {
             .heap_load_with_reg_offset(closure_register, free_variable_offset);
 
         let free_variable_offset = self.ir.sub(num_free_variables, counter);
-        let num_local = self.ir.load_from_memory(closure_register, 2);
+        let num_local = self.ir.load_from_memory(closure_register, 3);
         let num_local = self.ir.tag(num_local, BuiltInTypes::Int.get_tag());
         let free_variable_offset = self.ir.add(free_variable_offset, num_local);
         // // TODO: Make this better

src/gc/compacting.rs

@@ -340,6 +340,9 @@ impl CompactingHeap {
         }
 
         let size = *(pointer as *const usize) >> 1;
+        // TODO: We don't have a hard limit on obejct size
+        // right now. But anything this big is probably a mistake
+        assert!(size < 1000);
         let data = std::slice::from_raw_parts(pointer as *const u8, size + 8);
         let new_pointer = self.to_space.copy_data_to_offset(data);
         debug_assert!(new_pointer % 8 == 0, "Pointer is not aligned");

src/gc/simple_mark_and_sweep.rs

@@ -233,6 +233,8 @@ impl SimpleMarkSweepHeap {
     }
 
     fn add_free(&mut self, entry: FreeListEntry) {
+
+        assert!(entry.size <= self.space.segments[entry.segment].size, "Size is too big");
         // TODO: If a whole segment is free
         // I need a fast path where I don't have to update free list
 
@@ -421,6 +423,9 @@ impl SimpleMarkSweepHeap {
                             offset,
                             size: (data >> 1) + 8,
                         };
+                        // We have no defined hard cap yet.
+                        // but this is probably a bug
+                        assert!(entry.size < 1000);
                         let mut entered = false;
                         for current_entry in free_entries.iter_mut().rev() {
                             if current_entry.segment == entry.segment

src/ir.rs

@@ -865,7 +865,7 @@ impl Ir {
                     let register = alloc.allocate_register(index, dest, lang);
                     lang.mov_reg(register, lang.ret_reg());
                     for (index, register) in out_live_call_registers.iter().enumerate() {
-                        lang.pop_from_stack(*register, index as i32);
+                        lang.pop_from_stack_indexed(*register, index as i32);
                     }
                 }
                 Instruction::TailRecurse(dest, args) => {
@@ -920,8 +920,8 @@ impl Ir {
                     let dest = dest.try_into().unwrap();
                     let register = alloc.allocate_register(index, dest, lang);
                     lang.mov_reg(register, lang.ret_reg());
-                    for (index, register) in out_live_call_registers.iter().enumerate() {
-                        lang.pop_from_stack(*register, index as i32);
+                    for register in out_live_call_registers.iter().rev() {
+                        lang.pop_from_stack(*register);
                     }
                 }
                 Instruction::Compare(dest, a, b, condition) => {
@@ -1093,7 +1093,7 @@ impl Ir {
                 Instruction::PopStack(val) => {
                     let val = val.try_into().unwrap();
                     let val = alloc.allocate_register(index, val, lang);
-                    lang.pop_from_stack(val, 0);
+                    lang.pop_from_stack_indexed(val, 0);
                 }
                 Instruction::LoadFreeVariable(dest, free_variable) => {
                     let dest = dest.try_into().unwrap();

src/main.rs

@@ -195,9 +195,14 @@ pub unsafe extern "C" fn new_thread<Alloc: Allocator>(
     runtime: *mut Runtime<Alloc>,
     function: usize,
 ) -> usize {
-    let runtime = unsafe { &mut *runtime };
-    runtime.new_thread(function);
-    BuiltInTypes::null_value() as usize
+    #[cfg(feature = "thread-safe")] {
+        let runtime = unsafe { &mut *runtime };
+        runtime.new_thread(function);
+        BuiltInTypes::null_value() as usize
+    }
+    #[cfg(not(feature = "thread-safe"))] {
+        panic!("Threads are not supported in this build");
+    }
 }
 
 pub unsafe extern "C" fn __pause<Alloc: Allocator>(
@@ -251,7 +256,7 @@ fn compile_trampoline<Alloc: Allocator>(runtime: &mut Runtime<Alloc>) {
 
     lang.call(X10);
     // lang.breakpoint();
-    lang.pop_from_stack(X10, 0);
+    lang.pop_from_stack_indexed(X10, 0);
     lang.mov_reg(SP, X10);
     for (i, reg) in lang
         .canonical_volatile_registers
@@ -314,6 +319,12 @@ fn run_all_tests(args: CommandLineArguments) -> Result<(), Box<dyn Error>> {
         if !source.contains("// Expect") {
             continue;
         }
+        #[cfg(not(feature = "thread-safe"))]
+        {
+            if source.contains("// thread-safe") {
+                continue;
+            }
+        }
         println!("Running test: {}", path);
         let args = CommandLineArguments {
             program: Some(path.to_string()),

src/runtime.rs

@@ -1050,7 +1050,7 @@ impl<Alloc: Allocator> Runtime<Alloc> {
     ) -> Result<usize, Box<dyn Error>> {
         let len = 8 + 8 + 8 + free_variables.len() * 8;
         // TODO: Stack pointer should be passed in
-        let heap_pointer = self.allocate(len, 0, BuiltInTypes::Closure)?;
+        let heap_pointer = self.allocate(len / 8, 0, BuiltInTypes::Closure)?;
         let pointer = heap_pointer as *mut u8;
         let num_free = free_variables.len();
         let function_definition = self
@@ -1065,7 +1065,7 @@ impl<Alloc: Allocator> Runtime<Alloc> {
         let function = function.to_le_bytes();
 
         let free_variables = free_variables.iter().flat_map(|v| v.to_le_bytes());
-        let buffer = unsafe { from_raw_parts_mut(pointer, len) };
+        let buffer = unsafe { from_raw_parts_mut(pointer.add(8), len) };
         // write function pointer
         for (index, byte) in function.iter().enumerate() {
             buffer[index] = *byte;

src/types.rs

@@ -52,7 +52,8 @@ impl BuiltInTypes {
             0b100 => BuiltInTypes::Function,
             0b101 => BuiltInTypes::Closure,
             0b110 => BuiltInTypes::HeapObject,
-            _ => panic!("Invalid tag"),
+            0b111 => BuiltInTypes::Null,
+            _ => panic!("Invalid tag {}", pointer & 0b111),
         }
     }
 
@@ -121,4 +122,4 @@ fn tag_and_untag() {
         assert_eq!(kind, &BuiltInTypes::get_kind(tagged as usize));
         assert_eq!(value as usize, BuiltInTypes::untag(tagged as usize));
     }
-}
+}