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

enable softfloat on MIPS arch #2724

Merged
merged 1 commit into from Nov 7, 2018
Merged

enable softfloat on MIPS arch #2724

merged 1 commit into from Nov 7, 2018

Conversation

sedlund
Copy link
Contributor

@sedlund sedlund commented Nov 5, 2018

What is the purpose of this change?

MIPS does not have a floating point unit. The binaries built for MIPS do not run on devices that do not have MIPS_FPU enabled in kernel. Enabling GOMIPS=softfloat makes them work.

Was the change discussed in an issue or in the forum before?

Unsure

Checklist

  • I have read the contribution guidelines.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate.
  • All commit messages are in house style.
  • I'm done, this Pull Request is ready for review :-)

@Cnly
Copy link
Member

Cnly commented Nov 5, 2018

Hi @sedlund, it seems the GOARCH value you defined (MIPS) is not in https://golang.org/doc/install/source#environment. Can you confirm it is working for the target devices? Thanks!

@sedlund
Copy link
Contributor Author

sedlund commented Nov 5, 2018

sorry, I messed up the case of the GOARCH target. Updated, tested make compile_all. Binary for linux/mips works on my mips based device.

probably need to enable for mipsle as well (#2725) . I do not have a device to verify with though.

@ncw
Copy link
Member

ncw commented Nov 5, 2018

@sedlund this looks useful thank you :-)

  • Do you know what the consequences of enabling "softfloat" are? Does it mean that processors with floating point processors will use emulation?
  • I think it would be a good idea to do mipsle also
  • can you go fmt the file to fix the build please?

@sedlund
Copy link
Contributor Author

sedlund commented Nov 6, 2018

  1. From my understanding of patches from runtime: mips32 soft float point support golang/go#18162 it seems the softfloat flag causes the compiler to emit code that does not use hardware FPU. Which I think is different from emulation, as it is not translating calls, but using separate functions in place of the ones utilizing FPU.
  2. Added softfloat for mipsle
  3. Ran go fmt - travis passed, AppVeyor might have some other problem?

@ncw
Copy link
Member

ncw commented Nov 6, 2018

  1. From my understanding of patches from golang/go#18162 it seems the softfloat flag causes the compiler to emit code that does not use hardware FPU. Which I think is different from emulation, as it is not translating calls, but using separate functions in place of the ones utilizing FPU.

Fine, I think we can live with that as rclone isn't a heavy FPU user (just used for a few stats).

2. Added softfloat for mipsle

Thanks

3. Ran go fmt - travis passed, AppVeyor might have some other problem?

Yes the AppVeyor build is broken for different reasons!

I can't merge this using the github UI :-(

This branch cannot be rebased due to conflicts
Rebasing the commits of this branch on top of the base branch cannot be performed automatically due to conflicts encountered while reapplying the individual commits from the head branch.

I'm not sure what is causing the conflict - I think it is the merges you did. Merges in pull requests always seem to cause problems at github..

So can you squash your branch down to one commit, rebase it on master and force push it? That should allow me to merge cleanly and as a bonus it will pick up the fix for the AppVeyor CI!

Thanks

@Cnly
Copy link
Member

Cnly commented Nov 6, 2018

probably need to enable for mipsle as well (#2725) . I do not have a device to verify with though.

Just confirmed on a mipsle device provided by @tony-sunjian:

$ ./rclone -V
rclone v1.44-042-g4a6cfbeb-patch-1-beta
- os/arch: linux/mipsle
- go version: go1.11.2

MIPS does not have a floating point unit.  Enable softfloat to build binaries run on devices that do not have MIPS_FPU enabled in their kernel.
@sedlund
Copy link
Contributor Author

sedlund commented Nov 6, 2018

I think I defeated git finally. :) Let me know if it works for you.

@ncw
Copy link
Member

ncw commented Nov 7, 2018

You win your epic battle with git 🏆

Thank you very much. I'll merge that now :-)

@ncw ncw merged commit f139e07 into rclone:master Nov 7, 2018
@ivandeex ivandeex added the build label Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants