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

remove stdsimd feature flag #548

Merged
merged 6 commits into from
Feb 12, 2024
Merged

remove stdsimd feature flag #548

merged 6 commits into from
Feb 12, 2024

Conversation

jonaspleyer
Copy link
Contributor

@jonaspleyer jonaspleyer commented Feb 6, 2024

Problem

I was recently compiling the plotters crate with the nigthly compiler and got the following build error:

error[E0635]: unknown feature `stdsimd`
  --> /home/REDACTED/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pathfinder_simd-0.5.2/src/lib.rs:12:49
   |
12 | #![cfg_attr(pf_rustc_nightly, feature(simd_ffi, stdsimd))]
   |                                                 ^^^^^^^

For more information about this error, try `rustc --explain E0635`.

My nightly compiler is

# rustup show
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/REDACTEDrustup

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu
nightly-2022-12-18-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu (default)
1.58-x86_64-unknown-linux-gnu
1.75-x86_64-unknown-linux-gnu
1.62.0-x86_64-unknown-linux-gnu

active toolchain
----------------

nightly-x86_64-unknown-linux-gnu (default)
rustc 1.78.0-nightly (f067fd608 2024-02-05)

Solution

It seems that the feature flag stdsimd is not present in the nightly compiler anymore.
This patch very small patch is able to fix my problem and the package still compiles

diff --git a/simd/src/lib.rs b/simd/src/lib.rs
index cbffc2a4..89a087bd 100644
--- a/simd/src/lib.rs
+++ b/simd/src/lib.rs
@@ -9,7 +9,7 @@
 // except according to those terms.
 
 #![cfg_attr(pf_rustc_nightly, feature(link_llvm_intrinsics, platform_intrinsics))]
-#![cfg_attr(pf_rustc_nightly, feature(simd_ffi, stdsimd))]
+#![cfg_attr(pf_rustc_nightly, feature(simd_ffi))]
 
 //! A minimal SIMD abstraction, usable outside of Pathfinder.

with this stable compiler:

# rustup show
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/REDACTED.rustup

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu (default)
nightly-2022-12-18-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu
1.58-x86_64-unknown-linux-gnu
1.75-x86_64-unknown-linux-gnu
1.62.0-x86_64-unknown-linux-gnu

active toolchain
----------------

stable-x86_64-unknown-linux-gnu (default)
rustc 1.75.0 (82e1608df 2023-12-21)

Comments

I am aware that this patch affects stable upstream and do not know about the internals of this package. This patch might be obsolete depending on if the nightly version changes again in the future. I am looking forward on Feedback regarding this issue and this PR.
This may also be a nightly compiler error but I do not know how to check this.

@jayvdb
Copy link
Contributor

jayvdb commented Feb 11, 2024

@jdm @mrobinson @s3bk , could you take a look at this please.

@s3bk s3bk assigned s3bk and unassigned s3bk Feb 11, 2024
@jayvdb
Copy link
Contributor

jayvdb commented Feb 11, 2024

I noticed when zowens/crc32c#52 did this, they also re-added another feature for aarch64, see zowens/crc32c#54

@jonaspleyer
Copy link
Contributor Author

I have just confirmed that the build also works when including the following flags:

#![cfg_attr(all(target_arch = "aarch64", nightly), feature(stdarch_arm_crc32))]

I also upated the PR accordingly.

@s3bk
Copy link
Collaborator

s3bk commented Feb 12, 2024

I would say to merge this and see if it breaks for some architecture later?

simd/src/lib.rs Outdated
@@ -9,7 +9,8 @@
// except according to those terms.

#![cfg_attr(pf_rustc_nightly, feature(link_llvm_intrinsics, platform_intrinsics))]
#![cfg_attr(pf_rustc_nightly, feature(simd_ffi, stdsimd))]
#![cfg_attr(all(target_arch = "aarch64", nightly), feature(stdarch_arm_crc32))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this use pf_rustc_nightly or nightly ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The build seems to be working with both the nightly and pf_rustc_nightly flags. I tested with the following compilers:

active toolchain
----------------

nightly-2024-02-05-x86_64-unknown-linux-gnu (default)
rustc 1.78.0-nightly (268dbbbc4 2024-02-04)
active toolchain
----------------

beta-x86_64-unknown-linux-gnu (default)
rustc 1.77.0-beta.2 (f2048098a 2024-02-09)

However, the current nightly compiler fails again with a different error message.

active toolchain
----------------

nightly-x86_64-unknown-linux-gnu (default)
rustc 1.78.0-nightly (1a648b397 2024-02-11)

Error:

error[E0080]: could not evaluate static initializer
   --> /home/REDACTED/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/nonzero.rs:119:21
    |
119 |                     intrinsics::unreachable()
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^ entering unreachable code
    |
note: inside `NonZero::<u32>::new_unchecked`
   --> /home/REDACTED/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/num/nonzero.rs:119:21
    |
119 |                     intrinsics::unreachable()
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `Entry::new`
   --> /home/REDACTED/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pdf_encoding-0.1.0/src/lib.rs:109:17
    |
109 |                 NonZeroU32::new_unchecked(c as u32)
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `c`
   --> /home/REDACTED/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pdf_encoding-0.1.0/src/lib.rs:123:10
    |
123 |     Some(Entry::new(c))
    |          ^^^^^^^^^^^^^
note: inside `WINANSI`
   --> /home/REDACTED/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pdf_encoding-0.1.0/src/cp1252.rs:2:2
    |
2   |  c('\u{0000}'), c('\u{0001}'), c('\u{0002}'), c('\u{0003}'), c('\u{0004}'), c('\u{0005}'), c('\u{0006}'), c('\u{0007}'),
    |  ^^^^^^^^^^^^^

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh. Well, at least I can fix that easily.

@s3bk
Copy link
Collaborator

s3bk commented Feb 12, 2024

I am going to merge this and then make a PR to fix the pdf stuff.

@s3bk s3bk added this pull request to the merge queue Feb 12, 2024
Merged via the queue into servo:main with commit e4fcda0 Feb 12, 2024
2 checks passed
jonaspleyer added a commit to jonaspleyer/cellular_raza that referenced this pull request Feb 15, 2024
- due to bug in pathfinder which flows over to the plotters library
- see servo/pathfinder#548
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.

3 participants