search

rust-embedded / cortex-m-quickstart

Template to develop bare metal applications for Cortex-M microcontrollers
809 stars 167 forks source link

[RFC] recommend rust-lld as the default linker #39

Closed japaric closed 6 years ago

japaric commented 6 years ago

once the unstable flag -Z linker-flavor is not required

Advantages:

Disadvantages:

cc @rust-embedded/cortex-m

adamgreig commented 6 years ago

Seems fine to recommend lld as the default choice, especially with rust-embedded/cortex-m-rt#84 landed, and just mention in the docs (and the .cargo/config comments) how to use gcc instead and why you might want to.

korken89 commented 6 years ago

+1

therealprof commented 6 years ago

Am I missing something? I just removed the linker from my cargo config and it compiled just fine. What is preventing us from making the change now?

japaric commented 6 years ago

@therealprof the -Z linker-flavor flag which is required to use rust-lld is unstable and it's not clear if it will be stabilized for the edition release or if we'll land linker flavor inference (rust-lang/rust#52101) before the edition release.

therealprof commented 6 years ago

@japaric I misunderstood your proposal then: I thought rust-lld is the default now and we're recommending using gcc/binutils for linking. What is preventing us from using rust-lld as default?

japaric commented 6 years ago

@therealprof sorry, I think the RFC title could be more precise: "use rust-lld as the default linker".

For the thumb* targets rustc, by default, uses arm-none-eabi-gcc to link programs. quickstart v0.3.3 doesn't change the linker: rustc uses gcc as it normally would.

The proposal is to have a future quickstart release change the linker to rust-lld using .cargo/config. That means that if you clone quickstart and run cargo build --target $T you'd be using rust-lld.

What is preventing us from using rust-lld as default?

Changing the linker requires passing -Z linker-flavor to rustc. This flag is unstable and doesn't work on stable or beta. We have a policy that crates should work on stable (or beta) by default and that features that require nightly should be opt-in. Following that policy switching to rust-lld should be opt-in rather than the default.

therealprof commented 6 years ago

@japaric

What is preventing us from using rust-lld as default?

Changing the linker requires passing -Z linker-flavor to rustc. This flag is unstable and doesn't work on stable or beta. We have a policy that crates should work on stable (or beta) by default and that features that require nightly should be opt-in. Following that policy switching to rust-lld should be opt-in rather than the default.

Sorry for being so imprecise. I meant: What is preventing rustc from using lld by default? There should be a preference to using llvm parts instead of unrelated external depedencies, no?

japaric commented 6 years ago

What is preventing rustc from using lld by default?

Changing the default linker of any of the thumb* targets would be a breaking change (*). Imagine a user that only passes -Wl,-Tlink.x to the linker (via rustcflags); if we change the default linker from gcc to lld their build would break because lld expects -Tlink.x (w/o the -Wl, prefix).

(*) However, everyone who can build a binary is using nightly so it might be OK to do this breaking change without breaking Rust stability promise ("no breaking changes when doing updates on the stable channel). But, I imagine such change would require a rust-lang/rust RFC and getting an OK from all stakeholders. Also, there would need to be a stable mechanism to switch from rust-lld to arm-none-eabi-gcc so presumably either -Z linker-flavor would have to be stabilized or linker flavor inference would have to be implemented before changing the default linker.

therealprof commented 6 years ago

If we don't change it for Rust 2018 edition we'll be stuck with it forever (as approximation for "a long time"). I'd rather break it now instead of having to work around it for the foreseeable future. Unless of course a smarter mechanism is around the corner (*).

Fun fact: I've been using the binutils linker directly all the time (instead of going through gcc) and so the arguments to pass are exactly the same.

(*) Like introspecting the linker args and inferring which linker to use.

japaric commented 6 years ago

@alexcrichton what would it take (RFC or PR+FCP) to change the default linker of a built-in target? This is technically a breaking change but in this case the change would only break nightly users. More details in https://github.com/rust-embedded/cortex-m-quickstart/issues/39#issuecomment-411520041.

alexcrichton commented 6 years ago

An interesting question! It does sound unfortunatley like it'd be a breaking change, so in that sense to do this I think we'd just want to have a concrete handle on what exactly the breakage is (which it sounds like you've got) along with stakeholder buy-in that the breakage is worth it and/or there's a migration path.

In that sense this I think it's up to you whether it requires an RFC or not. The embedded working group likely embodies all the stakeholders here so if there's consenus to do this then it can likely be a PR!

A few things I've found helpful for things like this in the past are:

Does that make sense?