Major revisions

docs/C-Calling-Conventions.md

@@ -1,4 +1,4 @@
-# The Problem - Undefined Behavior calling Variadic C Functions
+# Undefined Behavior calling Variadic C Functions
 
 Our closures use the following declaration to store function pointers:
 
@@ -15,7 +15,7 @@ And in the closure object:
       object *elements;
     } closureN_type;
 
-This works fine to store closures. The problem is that because these function pointers do not precisely match the function definitions we run into undefined behavior when attempting to call them. On native platforms this is fine. But WASM platforms will throw an exception when calling variadic functions.
+This works fine to store closures. The problem is that because these function pointers do not precisely match the function definitions we run into undefined behavior when attempting to call them. On native platforms this is fine. But web assembly platforms will throw an exception when calling variadic functions.
 
 For example when calling a pointer to the following function we get a "function mismatch" error:
 
@@ -31,14 +31,25 @@ This is problematic for a few reasons:
 - The caller needs to know which cast to use. We can resolve that by extending the closure type and using a wrapper to call, however
 - The number of arguments in the pointer will differ depending on the variadic function we are wrapping. This creates the same problem we attempt to solve using `dispatch.c`. We can make this work for up to N arguments by using the same approach but that is clumsy at best.
 
+Ultimately this is not a viable solution. We need another way to avoid undefined behavior in the generated C code before we can get Cyclone programs running in web assembly.
+
 # Proposed Solution 
 
 The ideal solution is to change the signature of all of our C functions to a common interface so the function pointer may be called correctly in all cases.
 
+This calls back to [an earlier discussion](https://github.com/justinethier/cyclone/issues/193) on Cyclone's limit of ~128 function arguments:
+
+> CHICKEN scheme has a similar problem with regarding to the number of function arguments. They eventually solved the problem at the end of the 4.x series of releases by changing the function calling convention to always receive a vector as one of the arguments, instead of explicitly passing all arguments directly as C arguments. That works, perhaps with some overhead of vector packing/unpacking, but is a massive change. The good news is that it is mostly transparent to user code; if (cyclone foreign) is used no changes may even be required to existing applications.
+
+We can use a similar solution to avoid undefined behavior with the upside of allowing an unlimited (within reason) number of function arguments.
+
+The main obstacle to this is the scale of the change. We will need to specifiy a new C calling convention, modify the compiler to emit code using the new convention, and modify functions in the runtime accordingly. There may also be impacts to the FFI. It would be ideal if the existing FFI could be used without modification, we will have to see if that is reasonable. Even if `define-c` needs to use a different syntax for C functions it should be possible to modify `(cyclone foreign)` such that user code does not need to be modified.
+
+Once this is done there will no longer be a need for `dispatch.c`. Eliminating this module will give us a nice reduction in the size of `libcyclone.a` and generated executables.
 
 ## Current Function Signatures
 
-Our current function signature is as follows:
+It is important to first document and understand how the current system works before designing a new solution. Our current function signature is as follows:
 
     static void __lambda_1195(void *data, int argc, object closure, object k
 
@@ -64,22 +75,31 @@ TODO: `define-c` examples
       "(void *data, object ptr, object bv, object port, object start, object end)"
       " return Cyc_write_bytevector(data, bv, port, start, end);")
 
-## New Signatures
+## New Calling Convention
 
-TBD
+(TODO: still building this whole section)
 
+We want a signature similar to this:
 
     static void __lambda(void *data, object closure, object k, int argc, object *args) ;
 
+TODO:  better to pack args using a C array or a vector?
+
+advantages to vector:
+* can allocate vectors on the heap for extremely large numbers of arguments
+* vector specifies the number of arguments provided (is this any better than `argc` though?)
+
+disadvantages:
+- larger memory footprint
+- more data to pack (object header, etc)
+- more overhead to unpack?
+
 TODO: how to call these functions, need to pack args prior to call
 
 TODO: how to unpack args for a call. I think it would be simple, need to change compiler to point to members of `args` instead of directly to arguments
 
+* varargs functions still expect to receive extra arguments as part of a list. we may need to unpack the vector only to repackage all the args as a list
+
 TODO: fixed and variadic examples (same?)
 
-
-An added benefit of this, I think, is that we eliminate the restriction on number of function arguments.
-though, if this is on the stack there still is a limit
-
-TBD: no more need for dispatch.c?
-
+Finally, a key consideration here is runtime performance. We do not want this solution to significantly slow down our programs. I expect there may be some performance hit though. Again, this is an important consideration and we will need to run benchmarks to measure the impact. Adjustments will likely need to be made as part of this process.