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

Create mips64 soft float target #50902

Closed
wants to merge 2 commits into from

Conversation

jkilpatr
Copy link

@jkilpatr jkilpatr commented May 19, 2018

resolves #50890
probably missing some intermediate step here, I'm basing this off of #34922 but the first step seems deprecated?

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 19, 2018
@nagisa
Copy link
Member

nagisa commented May 20, 2018

Just like I said on the issue, the unknown-linux targets should stay hard float. For soft-float targets a new target should be introduced.

@jkilpatr
Copy link
Author

jkilpatr commented May 20, 2018

I agree conceptually, for maximum consistency you would want to get rid of

armv7-unknown-linux-gnueabihf
armv7-unknown-linux-musleabihf

and replace them with

armv7-unknown-linux-gnueabi-sf
armv7-unknown-linux-musleabi-sf
mips-unknown-linux-gnu-sf
mips-unknown-linux-musl-sf
mips64-unknown-linux-gnuabi64-sf
mips64el-unknown-linux-gnuabi64-sf
mipsel-unknown-linux-gnu-sf
mipsel-unknown-linux-musl-sf

That way the rule is 'all targets without -sf have hard float' instead of 'all targets, except some mips and arm ones expect hard float, but some arm ones also have a explicit hf version'. Less confusing for sure.

@jkilpatr
Copy link
Author

so how should I progress on this pull request?

@nagisa
Copy link
Member

nagisa commented May 21, 2018

IMO you should just add a new target with whatever new name. It doesn’t need to be very consistent with the ARM naming scheme, only something that could be used for all other mips targets. If there’s no prior art in other compilers, something like -muslsf would be just fine.

Ideally we’d just use something like -C soft-float and it would just work™, but sadly that’s not how it works with compiled code we distribute, so yeah, a target is well warranted at the moment.

cc @alexcrichton

@alexcrichton
Copy link
Member

cc @japaric, do you or others in the embedded working group have thoughts on this?

jkilpatr added 2 commits May 22, 2018 10:07
resolves rust-lang#50890

This commit also introduces a naming scheme for soft float targets
where 'sf' is appended to the end of the target to indicate a soft
float target. Leaving the default target as hard float.
@jkilpatr jkilpatr force-pushed the mips64-soft-float branch from b99f56c to 7ef6b7c Compare May 22, 2018 14:54
@jkilpatr
Copy link
Author

There we go, the musl target patch may belong in a different pull request though.

@jkilpatr jkilpatr changed the title Add soft float parameter to mips64 target Create mips64 soft float target May 22, 2018
@jkilpatr
Copy link
Author

how do I actually build these targets to test them out? I'm not seeing a function in x.py for it.

@paoloteti
Copy link
Contributor

If I'm not wrong compiler_builtins defaults to hard-float for MIPS, so +soft-float here is not enough.

@jkilpatr
Copy link
Author

@paoloteti if that's the case for mips32 as well as mips64 then I don't see any options for that in these files. Is there somewhere else I should be looking?

@paoloteti
Copy link
Contributor

@jkilpatr Yes look at: https://github.com/rust-lang-nursery/compiler-builtins/blob/master/build.rs
Similar to the eabihf case for ARM I think you need to set a 'soft-float' flag to the C toolchain and/or to skip some file.

@jkilpatr
Copy link
Author

@paoloteti so that would be a separate pull request on that repo? There is no reference to soft float in that file, so if setting the flag manually was required targets such as mips-unknown-linux-musl wouldn't work on devices without HF and I have half a dozen devices showing they do on hand.

Do you mean specifically mips64? Where would I find the spec outlining which targets have which defaults? somewhere in LLVM I assume.

@paoloteti
Copy link
Contributor

@jkilpatr my advice was just to check if the underline C toolchain for mips64 (mips64el-linux-gnuabi64-gcc) defaults or not to HF. I remember this 34910 for mips-gnu.

@alexcrichton
Copy link
Member

@jkilpatr do you know if there are preexisting conventions for mips targets? In general we try to follow Debian's guidelines and naming schemes, and do you know if they specify whether soft-float or hard-float is enabled?

@jkilpatr
Copy link
Author

@paoloteti @alexcrichton

Thanks, just needed to understand where the information was coming from. The answer is that mips64el-linux-gnuabi64 on Debian sid defaults to hard float. As do other mips targets.

root@2303509b5596:/# cat hello-world.c
#include <stdio.h>
int main()
{
   float pi = 3.14;
   float phi = 1.618;
   printf("%f{}", (pi + phi));
   return 0;
}
root@2303509b5596:/#` mips64el-linux-gnuabi64-objdump -d hello-world | grep lwc1
 b74:   c4400d98        lwc1    $f0,3480(v0)
 b80:   c4400d9c        lwc1    $f0,3484(v0)
 b88:   c7c10000        lwc1    $f1,0(s8)
 b8c:   c7c00004        lwc1    $f0,4(s8)

What's confusing me is that if we need a soft float flag in compiler builtins for mips64 to work with soft floats how in the world do the existing mips soft float targets work? They don't have anything like that in that file.

@paoloteti
Copy link
Contributor

@jkilpatr The key point is that mips64el-linux-gnuabi64 is hard-float only, so if you use -msoft-float the output will be something like this:

/usr/bin/mips64el-linux-gnuabi64-ld: Warning: a.out uses -mhard-float (set by /usr/lib/gcc-cross/mips64el-linux-gnuabi64/7/../../../../mips64el-linux-gnuabi64/lib/../lib/crt1.o), /tmp/ccTcsBcH.o uses -msoft-float

For mips-musl (mips-unknown-linux-musl) the C toolchain is mips-openwrt-linux-gcc that accept -msoft-float

@jkilpatr
Copy link
Author

@paoloteti in that case is it possible to make a -musl target rather than a gnuabi64 one? I don't see musl specific targets in the Debian package list so I assume they either A) come from somewhere else or B) are built using the -gnu cross compilers.

Also what does gnuabi64 mean anyways? Is it just to indicate a 64 bit version of gnu libc?

@paoloteti
Copy link
Contributor

@jkilpatr you are targeting a Cavium Octeon, so may be you can use the vendor toolchain (mips64-octeon-linux-gnu-gcc).

The root cause of your issue probably is that you have FPU emulation turned off.

OpenWrt userland is soft-float and Debian binaries are hard-float, but due to the fact that OpenWrt use soft-float the FPU emulation is turned off inside the kernel, so if you rebuild the kernel with the FPU emulation enabled, may be the issue will disappear.

@jkilpatr
Copy link
Author

I was successful using the mips FPU emulation option in the OpenWrt kernel config and building against glibc (musl tried to pass the soft float flag to the compiler with predictable results). I'll close this pull, but leave the issue open. At least until how to handle FP in cross targets becomes more standard.

@jkilpatr jkilpatr closed this May 27, 2018
@jkilpatr
Copy link
Author

an enormous thanks for your help @paoloteti

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mips64-unknown-linux-gnuabi64 assumes hard float support.
6 participants