-
Notifications
You must be signed in to change notification settings - Fork 16
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
MIPS Support #24
base: trunk
Are you sure you want to change the base?
MIPS Support #24
Conversation
A few notes about this PR (for when/if it's mergable):
|
Tenatively pushing a fix for that build error, need to test at home, but you're free to kick it off if you want :) |
Well that's something, one segfault, and one reproduction of the unwind failure. The segfault is concerning. 😄 |
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.
I think you need to revisit the assembly.
lw $s6, 0x58($a0) | ||
lw $s7, 0x5C($a0) | ||
lw $t8, 0x60($a0) | ||
lw $t9, 0x64($a0) |
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.
Where is the restoring code for at, k0, k1, fp, sp? Also please arrange everything in offset order.
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.
For the storing/restoring I mostly copied the riscv as reference, and directly mapped calls where appropriate. I'll get this fixed up though, wasn't sure what registers are critical. :)
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.
All non-volatile registers need to be saved, and all registers need to be restored.
src/unwinder/arch/mips.rs
Outdated
sw $gp, 0x70($sp) | ||
sw $v0, 0x08($sp) | ||
sw $v1, 0x0C($sp) | ||
sw $a0, 0x10($sp) |
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.
You can remove register saving for volatile registers.
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.
Still not removed
sw $s7, 0x5C($sp) | ||
sw $t8, 0x60($sp) | ||
sw $t9, 0x64($sp) | ||
" |
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.
Where's the saving code for sp?
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.
Darn, I meant to move it and made a blunder, will fix
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 not saving the correct sp. You have already clobbered it. It should be t0.
040dde1
to
477582b
Compare
There we go, fixed those errors, I omitted |
Alright, ready for another test I think. Decided to restore some asm I removed just to keep it inline with the other arches. Still encountering the stack unwind failure, but I think the asm is shored up. |
The restoration code still needs to restore volatiles, since personality function may choose to set them to certain values (although I think this rarely happens in practice). They can also be set by dwarf instructions. |
That one triggered an illegal instruction fault? Maybe I should avoid touching |
Absolutely amazing work. I'm really sorry you ended up needing to do the heavy lifting yourself, but thanks a million. I know this is super niche, and likely was a bit frustrating considering the lengthy back and forth. 🙏 Unfortunately it seems panics are still failing on the PSP itself, but now I can be mostly certain it's an issue with As for mips support itself, I'll take some time tomorrow evening and get this PR cleaned up (need to find the correct the fp32 P.S. it's not much, but I sent a sponsorship your way as thanks 🙂 |
a28d4fe
to
a832957
Compare
I noticed that your gimli PR was merged and I updated the PR to use new gimli types. CI is happy now, but it does spit a lot warnings that |
Hmm, yeah I recall having issues trying to pinpoint the clang cpu feature needed to gate double precision, and it might simply be a matter of cargo not supporting any of them at all. I can have a look again soon. 🙂 |
Draft commit to get to the bottom of MIPS troubles. 🙂