Compile without Make / migrate flags to .cargo/config.toml
#4054
Conversation
|
I don't understand how this works. What am I missing: The presence of the boards/.cargo directory has no impact on the built .bin file? |
|
@bradjc Yes, most of these optimization arguments actually have relatively little or no impact beyond LTO most of the time on the more recent compilers. |
|
@bradjc It also looks like you might be on a slightly different version. When I compile from |
ced02b2 to
98ed88a
Compare
|
So these flags don't do anything? Why are we setting them? On master if I make this diff: diff --git a/boards/Makefile.common b/boards/Makefile.common
index ab35928fa..3c276cd9a 100644
--- a/boards/Makefile.common
+++ b/boards/Makefile.common
@@ -64,9 +64,6 @@ TARGET_DIRECTORY ?= $(TOCK_ROOT_DIRECTORY)target/
# See https://github.com/rust-lang/rust/issues/60705 and
# https://github.com/tock/tock/issues/3529.
RUSTC_FLAGS ?= \
- -C link-arg=-Tlayout.ld \
- -C linker=rust-lld \
- -C linker-flavor=ld.lld \
-C relocation-model=static \
-C link-arg=-nmagic \
-C link-arg=-icf=all \I get: Which makes sense I think. But on commit 98ed88a if I entirely delete |
|
Don't remove the layout.ld line from the Makefile (this is an oversight on my branch, it's being set in build.rs now, so that line in the cargo toml is redundant, will fix) |
|
Specifically, on diff --git a/boards/Makefile.common b/boards/Makefile.common
index ab35928fa..045c8ec6b 100644
--- a/boards/Makefile.common
+++ b/boards/Makefile.common
@@ -65,12 +65,6 @@ TARGET_DIRECTORY ?= $(TOCK_ROOT_DIRECTORY)target/
# https://github.com/tock/tock/issues/3529.
RUSTC_FLAGS ?= \
-C link-arg=-Tlayout.ld \
- -C linker=rust-lld \
- -C linker-flavor=ld.lld \
- -C relocation-model=static \
- -C link-arg=-nmagic \
- -C link-arg=-icf=all \
- -C symbol-mangling-version=v0 \
# RISC-V-specific flags.
ifneq ($(findstring riscv32i, $(TARGET)),)before and after both: text data bss dec hex filename
112642 32 19864 132538 205ba /home/alevy/hack/tock/tock/target/thumbv7em-none-eabi/release/hail
1f03384de063e1b0ae85f753b5bdb13b859f187db4562053986ff05a1fc939a1 /home/alevy/hack/tock/tock/target/thumbv7em-none-eabi/release/hail.bin
sh |
Fixed in 33980aa |
|
Can you rebase? |
Useful for testing assigning ShortIds based on TBF headers.
|
Man.. 4 years old now: rust-lang/cargo#8244 The linked tracking issues ( rust-lang/cargo#12738 , rust-lang/cargo#12739 ) seem like they would help alleviate a lot of this, but those have a lot of open issues and not a lot of progress :( |
|
@lschuermann @bradjc out of tree, it is not necessary to have two separate |
|
IIUC, can we now pass a custom path for That would be a best of both worlds (... I suppose only assuming that this custom path can be specified in the |
|
To be clear, to everyone reading, the So the real lesson here is that we need almost no machinery to compile a board with Cargo, except to the extent that we include optimizations automatically without making them explicit on the command line. My contention is that the Makefile system obscures this ( |
This is true until it isn't. Things will break eventually without the -nmagic flag, for example, and no one wants to be debugging that. When I asked if these flags were needed in an earlier post, I was questioning if perhaps they became the default.
This is fair, with that one pretty significant caveat (nmagic). In general this is one of my major concerns, critical flags that get hidden/lost amid a general sense of those flags are just optimizations, and then when someone doesn't use them they spend a long time chasing bugs due to the linker. I've opened multiple PRs to move some of the makefile flags to build.rs, and closed them all because in the end it is really nice having all of the magic in one place.
I think we should not have called it Makefile.common so we could have syntax highlighting on by default. Alas. I'm fairly convinced of the opposite. Make allows us to express the actual complexity of building a project like this in one place. With rust/cargo, we have to spread the same logic over two subsystems (.cargo/config and build.rs) and an additional tool (copy-and-paste to all of the board .cargo files, at least for riscv). I would be more convinced we should do all of this in build.rs because we need the ability to use
(When I say build.rs I mean what libtock-rs has, where there is a build-helper crate and each board can call the provided functions.) I don't like that I'm writing all of this. But, frankly, cargo is a tool built for users running rust on their host machine, and happens to have enough support for other workflows so it kind of works. I just don't think it is expressive enough for our needs. |
I quite like this idea, but I don't believe it would cover our |
| lto = false | ||
| lto = true |
There was a problem hiding this comment.
What does this do? How does this relate to
# Enable link-time-optimization
"-C", "lto",
?
There was a problem hiding this comment.
"-C", "lto" is redundant and should be removed. If you set lto = false but then pass "-C", "lto", you get a compile error on debug builds.
|
|
||
| [unstable] | ||
| unstable-options = true | ||
| build-std = ["core", "compiler_builtins" ] |
There was a problem hiding this comment.
Can you add the newlines so we don't have these circles everywhere?
There was a problem hiding this comment.
It seems like Cargo automatically removes them when it touches these files.
There was a problem hiding this comment.
That's not what I'm seeing?
❯ cd ../../teensy40
❯ git diff | cat
diff --git a/boards/teensy40/.cargo/config.toml b/boards/teensy40/.cargo/config.toml
index 956e5d913..7bb661092 100644
--- a/boards/teensy40/.cargo/config.toml
+++ b/boards/teensy40/.cargo/config.toml
@@ -7,4 +7,8 @@ target = "thumbv7em-none-eabi"
[unstable]
unstable-options = true
-build-std = ["core", "compiler_builtins" ]
\ No newline at end of file
+build-std = ["core", "compiler_builtins" ]
+
+
+
+
❯ cargo build
Compiling teensy40 v0.1.0 (/Users/bradjc/git/tock/boards/teensy40)
Compiling cortexm7 v0.1.0 (/Users/bradjc/git/tock/arch/cortex-m7)
Compiling imxrt10xx v0.1.0 (/Users/bradjc/git/tock/chips/imxrt10xx)
Finished `dev` profile [optimized + debuginfo] target(s) in 6.93s
❯ git diff | cat
diff --git a/boards/teensy40/.cargo/config.toml b/boards/teensy40/.cargo/config.toml
index 956e5d913..7bb661092 100644
--- a/boards/teensy40/.cargo/config.toml
+++ b/boards/teensy40/.cargo/config.toml
@@ -7,4 +7,8 @@ target = "thumbv7em-none-eabi"
[unstable]
unstable-options = true
-build-std = ["core", "compiler_builtins" ]
\ No newline at end of file
+build-std = ["core", "compiler_builtins" ]
+
+
+
+
| let rtt_memory = kernel::static_buf!(capsules_extra::segger_rtt::SeggerRttMemory); | ||
| let rtt_memory = | ||
| kernel::static_buf!(capsules_extra::segger_rtt::SeggerRttMemory, "_SEGGER_RTT"); |
There was a problem hiding this comment.
This is a different change to go with the static_buf PR.
| rustflags = [ | ||
| # Use the LLVM lld executable with the `-flavor gnu` flag. | ||
| "-C", "linker-flavor=ld.lld", | ||
| # Use static relocation model. See https://github.com/tock/tock/pull/2853 | ||
| "-C", "relocation-model=static", |
There was a problem hiding this comment.
What happened to -C linker=rust-lld?
I get
❯ cargo build
Compiling nrf52840dk v0.1.0 (/Users/bradjc/git/tock/boards/nordic/nrf52840dk)
Compiling nrf5x v0.1.0 (/Users/bradjc/git/tock/chips/nrf5x)
Compiling cortexm4 v0.1.0 (/Users/bradjc/git/tock/arch/cortex-m4)
Compiling nrf52 v0.1.0 (/Users/bradjc/git/tock/chips/nrf52)
Compiling nrf52_components v0.1.0 (/Users/bradjc/git/tock/boards/nordic/nrf52_components)
Compiling nrf52840 v0.1.0 (/Users/bradjc/git/tock/chips/nrf52840)
error: linker `lld` not found
|
= note: No such file or directory (os error 2)
error: could not compile `nrf52840dk` (bin "nrf52840dk") due to 1 previous error
Ok, so in some sense this does work. If we create a And run
But...that doesn't work for me. Ex: (no extra flags, yet the config-include is set to true.) Contrast with: So, since they did not stabilize this, I don't think this is viable unfortunately. Unless of course I'm doing something wrong. This would, of course, be really useful. All of the riscv boards could include a riscv config that includes the riscv flags. Alas. |
|
I think to make this PR work we need rust-lang/cargo#7723 (comment) fixed. I'm by no means familiar with the cargo source, but I think this |
|
I have a strong distaste for including essential build information that end-users run (as opposed to github actions, for example) in hidden files. This configuration is as important for actually running Tock as any line of code is, and hiding configuration from users adds just another barrier for understanding how Tock works with no benefit. With that clearly stated, for better or worse 1) cargo is defacto in the Rust community and 2) it only supports hidden configuration files. Given that, I think with enough upside we should take a step backwards in usability by using hidden configuration files. In my opinion, we need two features for there to be enough upside:
Item (1) is the point of this PR so we are good there. Item (2) is harder. Logically, it seems like build.rs is designed for this (i.e., write actual code to determine how to build a particular executable), but in practice I don't think it is possible with build.rs. As such, we currently use make to implement the conditions we need. Instead, I think that the cargo config-include feature is enough to give us the flexibility to conditionally include flags for different boards. Rather than an So, I support this PR if we can get cargo config-include to work. |
|
We talked about this a while on the mini-core call earlier, and I've also ceded to a universe where the only things in the hidden file are (1) which target, and (2) which non-hidden config's to include. |
|
Created a PR to cargo, to have it respect the |
With rust-lang/cargo#14196, I believe we should be able to pull a new Rust nightly in a couple days and that condition should be satisfied. |
|
I just tried this with today's nightly and can confirm that something like: does work as we want. |
FWIW, I think that recursive includes should work as well now. |
.cargo/config.toml
|
Tried using The only issue i ran into was this warning: |
|
FYI I'm working on this PR |
|
I think this is the fix for remap-paths: 7e2ed4f |
|
Blocked in favor of #4075 (assuming that merges we'll close this) |
Pull Request Overview
Adds support to all boards for building using plain-old-cargo in an identical way to using the Makefile infrastructure.
Without this change, in order to build with
cargo, one needs a large number ofRUSTCFLAGSalong with cargo-specific arguments. With this change, simply:$ cd boards/[BOARD-OF-CHOICE] $ cargo build --releaseUses exactly the same RUSTFLAGS arguments, linker, linker arguments etc.
One additional argument (
-Ztrim-paths) results in identical binaries by remapping paths:This is done by adding two layers of
.cargo/config.tomlfiles:boards/.cargo/config.toml, which specifies non-target-specificRUSTFLAGS.cargo/config.tomlthat specifies the default target for building with cargo, and the-Zbuild-stdoption to recompile core, as that it is a board-specific choice (not possible for boards we want to compile with stable Rust)With this change, it is also easy to use tools such as
cargo-binutilsandprobe-rswhich rely on being able to runcargo buildwith normal arguments. For example, in the sma_q3 board, it is now possible to flash and attach the JLink RTT by simply installingprobe-rsand running:Testing Strategy
I manually compiled each board using
makeandcargo build -Ztrim-paths --releaseand validated that text, bss, and data segments are identical sizes and sampled that several boards function in both cases by flashing them. Though this should have no impact on normalmake-based workflows.TODO or Help Wanted
N/A
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.