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

added section on cranelift codegen #801

Closed
wants to merge 2 commits into from
Closed

Conversation

MalekiRe
Copy link

@MalekiRe MalekiRe commented Nov 4, 2023

No description provided.

@paul-hansen
Copy link
Contributor

paul-hansen commented Nov 9, 2023

Idk how big of a deal it is, but the configuration the blog post this PR suggests doesn't work in projects that need to support Wasm. This is because per-target profiles aren't supported by Rust You could create a new profile to handle this but then you have to specify the profile every time.

Instead it might be ideal to use rustflags for this as they can be specified per platform. Making cranelift still be used for linux but not for Wasm builds:

Cranelift

rustup component add rustc-codegen-cranelift-preview --toolchain nightly

~/.cargo/config.toml

[target.x86_64-unknown-linux-gnu]
rustflags = ["-Zcodegen-backend=cranelift", ]

Cranelift + Mold

~/.cargo/config.toml

[target.x86_64-unknown-linux-gnu]
rustflags = ["-Zcodegen-backend=cranelift", "-C", "link-arg=-fuse-ld=/usr/bin/mold"]
linker = "clang"

Thanks to this blog post for helping me discover this as an option: https://benw.is/posts/how-i-improved-my-rust-compile-times-by-seventy-five-percent

@paul-hansen
Copy link
Contributor

paul-hansen commented Nov 9, 2023

Though you wouldn't want to start shipping builds compiled with cranelift (for performance reasons) so maybe it's not a good idea to recommend that as it isn't limited to dev builds and people might forget to take it out. Personally I do my release builds in github actions so that wasn't a concern for me.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review Ready for a maintainer to consider for merging label May 5, 2024
@alice-i-cecile
Copy link
Member

@MalekiRe if you rebase this I'll merge it in :)

@alice-i-cecile alice-i-cecile added the S-Adopt-Me This PR could use some love: make a new PR with this work! label May 13, 2024
@@ -200,6 +200,9 @@ Bevy can be built just fine using default configuration on stable Rust. However

For more information, see [The rustup book: Overrides](https://rust-lang.github.io/rustup/overrides.html#the-toolchain-file).

* **Cranelift Codegen**: This uses a new nightly-only codegen that can be 300% faster than LLVM! However it only currently works on Linux and is generally still immature.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

30% is a more realistic average.

It works fine on Windows now and x86_64 macOS (arm64 macOS has some ABI issues)

Copy link
Member

@janhohenheim janhohenheim Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not get Bevy compiling on (intel) macOS with cranelift enabled. Specifically objc2 failed to build.
Windows works, but the installation process is a bit whackier and it may not play nice with sccache.
After my little investigation, it seems like the only platform where everything runs smoothly out of the box is still Linux.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I'll post one tomorrow :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: there you go rust-lang/rustc_codegen_cranelift#1504
Let me know in that issue if there's anything I can try out to help debug it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@@ -200,6 +200,9 @@ Bevy can be built just fine using default configuration on stable Rust. However

For more information, see [The rustup book: Overrides](https://rust-lang.github.io/rustup/overrides.html#the-toolchain-file).

* **Cranelift Codegen**: This uses a new nightly-only codegen that can be 300% faster than LLVM! However it only currently works on Linux and is generally still immature.
[Cranelift codegen setup](https://bjorn3.github.io/2023/10/31/progress-report-oct-2023.html).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe link to the setup section of the readme instead? Unlike this page it will be kept up to date if the setup changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it is possible to compile all dependencies with the LLVM backend and optimizations, but the main game crate with Cranelift to get much better runtime performance.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjorn3 I've tried that before and ran into issues where I would get random segfaults.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were some abi incompatibilities in the past, but most should be fixed now.

@BD103 BD103 removed the S-Ready-For-Final-Review Ready for a maintainer to consider for merging label Jun 11, 2024
@alice-i-cecile alice-i-cecile added the X-Contentious There are nontrivial implications that should be thought through label Jun 21, 2024
@janhohenheim janhohenheim mentioned this pull request Jul 4, 2024
3 tasks
@janhohenheim
Copy link
Member

Closing in favor of #1523

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Book C-Content S-Adopt-Me This PR could use some love: make a new PR with this work! X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants