Don't enable SSE for wasm compilation #7650
Assignees
Labels
T-contract-runtime
Team: issues relevant to the contract runtime team
Comments
matklad
added
the
T-contract-runtime
|
Status: this is fixed in Cargo, so we probably should just wait 10-ish weeks for fix to land the stable and then revert the revert. If someone feels strongly about this, we can try to apply some work-around faster. |
|
I don't think this is pressing. We can wait for a clean fix. |
|
Looks like rust-lang/cargo#11114 was now merged and the fix is planned to be released as part of 1.66 cargo release. |
it seems like 1.66 was released and we can proceed with this |
near-bulldozer bot
pushed a commit
that referenced
this issue
Jan 6, 2023
https://blog.rust-lang.org/2022/12/15/Rust-1.66.0.html Updated the same files as in the previous version bump: #7993 This enables #7650 (see [this comment](#7650 (comment))). Also includes fixes for the following warning: ``` warning: for loop over an `Option`. This is more readably written as an `if let` statement ... = note: `#[warn(for_loops_over_fallibles)]` on by default ```
near-bulldozer bot
pushed a commit
that referenced
this issue
Jan 9, 2023
) With #8284 we upgraded cargo version to `1.66`, so rust-lang/cargo#11114 is available. This PR is the same as #7130, but we can safely merge it now. See #7650 for more context. In order to test it I've used the code from [`librocksdb-sys/build.rs`](https://github.com/rust-rocksdb/rust-rocksdb/blob/master/librocksdb-sys/build.rs#L111) as part of `neard/build.rs`: ``` fn main() { - if let Err(err) = try_main() { - eprintln!("{}", err); - std::process::exit(1); + let target_feature = std::env::var("CARGO_CFG_TARGET_FEATURE").unwrap(); + let target_features: Vec<_> = target_feature.split(',').collect(); + if target_features.contains(&"neon") { + panic!("FAIL: {target_feature}"); + } else { + panic!("OK: {target_feature}"); } } ``` It was was tested on M1 macbook, so I've changed the feature to `neon` (which is enabled by default), my `.cargo/config.toml` is as follows: ``` -[target.'cfg(target_arch = "x86_64")'] -rustflags = ["-Ctarget-feature=+sse4.1,+sse4.2", "-Cforce-unwind-tables=y"] +[target.'cfg(target_arch = "aarch64")'] +rustflags = ["-Ctarget-feature=-neon", "-Cforce-unwind-tables=y"] ``` `neon` was successfully removed when using `1.66` toolchain: ``` OK: crc,...,vh ``` while it was still present with `1.65`: ``` FAIL: aes,..,neon,...,vh ``` This proves that rustflags are passed as env variable to `build.rs` in `1.66`, which was not the case in `1.65`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In #7130 we had disabled setting SSE for WASM compilation which helped in removing a bunch of warnings for WASM compilation.
We discovered that this PR causes some performance regressions (in effect it actually disabled SSE for x86 compilation as well) and did some investigation in rust-lang/cargo#11114. For now we have reverted the above PR. This issue is to track the work to disable setting SSE for WASM in a way that does not cause a performance regression.
CC: @matklad, @mm-near
The text was updated successfully, but these errors were encountered: