-
Notifications
You must be signed in to change notification settings - Fork 63
Stop __set_heap_size from trashing soft stack #410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
__set_heap_limit now sanity checks against stack; users can now grab all memory for heap mos-sim flushes every output character -- no waiting for output These changes were necessary to be able to get pi.c to calculate to 50000 digits on the Commodore 64. Memory allocation had a tendency to walk all over the stack; this puts some sanity checks in place to prevent others from doing the same. Default behavior is still 4 KB for heap.
With this change, a user can:
and they will get the maximum amount of platform memory available for malloc(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a really nice change! Few comments, otherwise LGTM!
@@ -179,6 +193,7 @@ FreeChunk *find_fit(size_t size) { | |||
return nullptr; | |||
} | |||
|
|||
" rts\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely a typo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Get current soft stack pointer value | ||
__asm__ ( | ||
".text\n" | ||
".global __get_current_stack_top\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can avoid making this a global symbol at all by using inline assembly in a static function. This would also allow the compiler to inline this, which would usually be beneficial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but inline assembly WITHIN a function is a bit of a dark art -- let me try...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is def optional; lemme know if you wanna take a crack at this later, and I"ll merge.
Limit the __set_heap_size function to not trash the stack if too much memory is requested. Additionally, choose a default stack size and use it as a safety margin when increasing heap size.
The new stack size, __stack_reserve_size, is set weakly to 1024, and anyone who wants to change it can do so without recompiling.
Additionally, the simulator now flushes on stdout output. This helps establish that long-running tasks are still running on mos-sim.
These changes were necessary to be able to get pi.c to calculate to 50000 digits on the Commodore 64.