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

Redox x86_64 target support #38366

Closed
wants to merge 6 commits into from
Closed

Conversation

jackpot51
Copy link
Contributor

This adds support for the x86_64 architecture of Redox to rustc. This patch will generate #[no_std] executables and, following the merge of #37702, the cross compiling of the standard library will be possible.

This differs from other targets in that it does not link libc, as Redox does not have a dependency on libc.

It is possible that libopenlibm will be added to post_link_args for math functions in the future.

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

@jackpot51 jackpot51 changed the title Redox target support Redox x86_64 target support Dec 14, 2016
@brson
Copy link
Contributor

brson commented Dec 15, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Dec 15, 2016

📌 Commit dc2b3fb has been approved by brson

@brson
Copy link
Contributor

brson commented Dec 15, 2016

@bors r- Oh, I see this defines a redox target_family. Let's please not do that. Right now the only "families" are "windows" and "unix" and I'm not inclined to change that.

@brson
Copy link
Contributor

brson commented Dec 15, 2016

Oh, this is weird. The code that sets up the value of target_family in default_configurations assumes that anything without a target_family is "unix", which isn't right here. Hm, I'm not sure what to do. What we're doing now is definitely wrong:

https://github.com/rust-lang/rust/blob/master/src/librustc/session/config.rs#L946

    let fam = if let Some(ref fam) = sess.target.target.options.target_family {
        Symbol::intern(fam)
    } else if sess.target.target.options.is_like_windows {
        Symbol::intern("windows")
    } else {
        Symbol::intern("unix")
    };

    let mut ret = HashSet::new();
    ret.insert((Symbol::intern("target_family"), Some(fam)));
    if fam == "windows" || fam == "unix" {
        ret.insert((fam, None));
    }

What I expected this code to be doing was more like:

    let fam = sess.target.target.options.target_family;
    if let Some(fam) {
        ret.insert((Symbol::intern("target_family"), Some(fam)));
        ret.insert((fam, None));
    }

With the target specs actually defining the family used by cfg(target_family), and cfg(windows) and cfg(unix) being based off the target_family. But that's not what's happening - today no target specifies target_family; it's all inferred during session creation.

I think my preference is to modify the existing code to work as I described, so that target_family is simply not defined for plattforms that are not windows and unix. This is mildly breaking for custom targets, but probably wouldn't be noticed.

@brson
Copy link
Contributor

brson commented Dec 15, 2016

f? @rust-lang/lang my comment above about the definition of cfg(target_family). It seems wrong today, assuming all plattforms are either windows or unix.

@japaric
Copy link
Member

japaric commented Dec 15, 2016

But that's not what's happening - today no target specifies target_family; it's all inferred during session creation.

related issue: #38187

@jackpot51
Copy link
Contributor Author

jackpot51 commented Dec 15, 2016

@brson I would be happy to not specify target_family if cfg(unix) wasn't set in doing so as you have seen, breaking the Redox build.

I believe, based on the if is_like_windows { windows } else { unix } statement above, that having a redox target_family is the cleanest solution right now, as the other platforms do things like is_like_osx, is_like_solaris, is_like_linux and do not set target_family to unix. This means that the dozen or so Unix targets would have to have target_family set to switch to what you have suggested, and I believe Unix is a fair assumption as a default target family.

Redox is Unix-like, but cannot be cfg(unix) as that implies a POSIX compliant C library.

So far I have not run into any build breaking things by having a target_family other than unix and windows.

@alexcrichton
Copy link
Member

r? @brson

@rust-highfive rust-highfive assigned brson and unassigned pnkfelix Dec 15, 2016
@jackpot51
Copy link
Contributor Author

A more complete change is here: #38401

@jackpot51 jackpot51 closed this Dec 16, 2016
bors added a commit that referenced this pull request Dec 23, 2016
Redox Cross Compilation

I will admit - there are things here that I wish I did not have to do. This completes the ability to create a cross compiler from the rust repository for `x86_64-unknown-redox`. I will document this PR with inline comments explaining some things.

[View this gist to see how a cross compiler is built](https://gist.github.com/jackpot51/6680ad973986e84d69c79854249f2b7e)

Prior discussion of a smaller change is here: #38366
@jackpot51 jackpot51 deleted the redox_target branch December 23, 2016 14:18
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.

7 participants