-
Notifications
You must be signed in to change notification settings - Fork 11
Fix/various fixes and refactoring #34
Fix/various fixes and refactoring #34
Conversation
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.
Looks fine to me as far as I can tell, but I won't pretend I understand all of the assembly 😅
@MabezDev would you be able to take a look at this as well, please?
LGTM! Just for my understanding, SPILL_REGISTERS uses WOE (WindowOverFlow) mechanism to spill the registers onto the stack, and in doing so clobbers a0-a3? Also why is |
Yes SPILL_REGISTERS uses WOE for spilling the registers. We use the same approach as IDF here - there is another way to do it without WOE used by NuttX I think - that code is huge and slower Spilling the registers doesn't clobber A0-A3 but the code to prepare and restore PS and EPC1 does The thing about Will look into that - probably won't get to it today anymore |
A stack frame size of 256 seems to be totally fine - thanks for pointing this out. Seems I got very confused about that original code comment ... and the windowed ABI is still a bit confusing to me at least when interrupts and exceptions come into play So, my understanding is: In extreme it will spill up to 8 registers at the "top" of the stack frame (32 bytes which matches the extra space of 0x20 reserved by IDF and NuttX). I copied some more comments from IDF (and added some more) to hopefully make things a bit less confusing for future readers of the code - including me |
@MabezDev would you mind checking the newly added comments - if they make sense and make the code more understandable? If everyone is fine with this I'll merge and publish the changes |
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.
LGTM! Thanks for the comments, I'm starting to get my head around the windowed ABI now :D
This fixes #33
Problems were
Additionally, this contains some refactoring
.ifc
usage and makes it easier to understand IMHO)Also, this prepares some things we'll need to get preemtive task-switching to work (to start working on using the WiFi drivers, that will also need some minor changes to the HAL - will come when this is available on crates.io)
Finally, I bumped the version of this and
core-isa-parser
(and updated some versions of dependencies)