Skip to content
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

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft

MIPS Support #24

wants to merge 2 commits into from

Conversation

SK83RJOSH
Copy link

@SK83RJOSH SK83RJOSH commented Jan 10, 2024

Draft commit to get to the bottom of MIPS troubles. 🙂

@SK83RJOSH
Copy link
Author

SK83RJOSH commented Jan 10, 2024

A few notes about this PR (for when/if it's mergable):

  • We should upstream Arch definitions to gimli (assuming they're interested)
  • Similar to RISCV and I think the FP1..=32 registers are optional depending on configuration; need to figure out correct target_features to support them correctly
  • Depending on the configuration, I think there can be far more total registers, GP/FP support should be enough for things generally

@SK83RJOSH
Copy link
Author

Tenatively pushing a fix for that build error, need to test at home, but you're free to kick it off if you want :)

@SK83RJOSH
Copy link
Author

Well that's something, one segfault, and one reproduction of the unwind failure. The segfault is concerning. 😄

Copy link
Owner

@nbdd0121 nbdd0121 left a 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)
Copy link
Owner

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.

Copy link
Author

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. :)

Copy link
Owner

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.

sw $gp, 0x70($sp)
sw $v0, 0x08($sp)
sw $v1, 0x0C($sp)
sw $a0, 0x10($sp)
Copy link
Owner

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.

Copy link
Owner

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)
"
Copy link
Owner

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?

Copy link
Author

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

Copy link
Owner

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.

@SK83RJOSH
Copy link
Author

There we go, fixed those errors, I omitted $sp and $fp somewhere along the way which... would definitely explain a lot. My apologies, I was definitely getting tunnel vision when I made those changes last night.

@SK83RJOSH
Copy link
Author

SK83RJOSH commented Jan 10, 2024

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.

@nbdd0121
Copy link
Owner

nbdd0121 commented Jan 10, 2024

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.

@SK83RJOSH
Copy link
Author

That one triggered an illegal instruction fault? Maybe I should avoid touching $at, $k0, $k1 out right? Or unrelated you think? 🙂

@SK83RJOSH
Copy link
Author

SK83RJOSH commented Jan 11, 2024

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 lld itself or the combination of mips and fde-static. Would it make sense to add a test for non-libc targets? I suppose the tricky thing about that would be providing the __eh_frame symbol in a way that would work across the target matrix.

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 target_feature + open a PR with gimli + add support for -fno-oddregs; which should all be straight forward enough)

P.S. it's not much, but I sent a sponsorship your way as thanks 🙂

@nbdd0121 nbdd0121 mentioned this pull request Jan 11, 2024
@nbdd0121 nbdd0121 force-pushed the mips branch 3 times, most recently from a28d4fe to a832957 Compare June 17, 2024 14:30
@nbdd0121
Copy link
Owner

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 single-float feature doesn't exist.

@SK83RJOSH
Copy link
Author

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. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants