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

Since nightly-2024-02-06 , could not compile with --features 'simd-accel' #2748

Closed
1 task done
PierreAntoineGuillaume opened this issue Mar 7, 2024 · 4 comments · Fixed by #2749
Closed
1 task done

Comments

@PierreAntoineGuillaume
Copy link

PierreAntoineGuillaume commented Mar 7, 2024

Please tick this box to confirm you have reviewed the above.

  • I have a different issue.

What version of ripgrep are you using?

6ebebb2 (current master HEAD, few commits after 14.1.0)

How did you install ripgrep?

Trying to compiling it from source

What operating system are you using ripgrep on?

Ubuntu 20.04.6 LTS

Describe your bug.

Cannot compile

What are the steps to reproduce the behavior?

RUSTUP_TOOLCHAIN=nightly-2024-02-06 cargo build --features 'simd-accel' fails
RUSTUP_TOOLCHAIN=nightly-2024-02-05 cargo build --features 'simd-accel' works

Because:

   --> ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/packed_simd-0.3.9/src/lib.rs:218:5
    |
218 |     platform_intrinsics,
    |     ^^^^^^^^^^^^^^^^^^^ feature has been removed
    |
    = note: SIMD intrinsics use the regular intrinsics ABI now

Then in later nightly version :

invalid ABI: found `platform-intrinsic`

related: rust-lang/rust@ea37e80
rust-lang/rust#117372

I don't know if this is actually a bug.

What is the actual behavior?

Cannot compile

What is the expected behavior?

Can compile

@PierreAntoineGuillaume
Copy link
Author

This is more a report and a reference than a real bug, and I will be trying to make a pull request but anything simd-related is way above my knowledge.

@BurntSushi
Copy link
Owner

As documented in the README, simd-accel requires a Rust nightly compiler because it relies on unstable features. As also documented in the README, the only thing it helps is UTF-16 decoding. So you might want to re-evaluate why you're enabling it in the first place.

I am considering just removing the simd-accel feature because I am absolutely tired of the fact that encoding_rs refuses to use stable APIs for its SIMD features. All of ripgrep's SIMD optimizations for search do not require it. They use stable but platform specific APIs.

In any case, I'm not tracking this issue here. There's an issue for it at the source: rust-lang/packed_simd#359

@BurntSushi BurntSushi closed this as not planned Won't fix, can't repro, duplicate, stale Mar 7, 2024
BurntSushi added a commit that referenced this issue Mar 7, 2024
This feature causes nothing but problems and is frequently broken. The
only optimization it was enabling were SIMD optimizations for
transcoding. In particular, for UTF-16 transcoding. This is performed by
the [`encoding_rs`](https://github.com/hsivonen/encoding_rs) crate,
which specifically uses unstable portable SIMD APIs instead of the
stable non-portable SIMD APIs.

SIMD optimizations that apply to search have long been making use of
stable APIs, and are automatically enabled when your target supports
them. This is, IMO, the correct user experience and one that
`encoding_rs` refuses to support. I'm done dealing with it, so
transcoding will only use scalar code until the SIMD optimizations in
`encoding_rs` work on stable. (This doesn't mean that `encoding_rs` has
to change. This could also be fixed by stabilizing `std::simd`.)

Fixes #2748
BurntSushi added a commit that referenced this issue Mar 7, 2024
This feature causes nothing but problems and is frequently broken. The
only optimization it was enabling were SIMD optimizations for
transcoding. In particular, for UTF-16 transcoding. This is performed by
the [`encoding_rs`](https://github.com/hsivonen/encoding_rs) crate,
which specifically uses unstable portable SIMD APIs instead of the
stable non-portable SIMD APIs.

SIMD optimizations that apply to search have long been making use of
stable APIs, and are automatically enabled when your target supports
them. This is, IMO, the correct user experience and one that
`encoding_rs` refuses to support. I'm done dealing with it, so
transcoding will only use scalar code until the SIMD optimizations in
`encoding_rs` work on stable. (This doesn't mean that `encoding_rs` has
to change. This could also be fixed by stabilizing `std::simd`.)

Fixes #2748
@BurntSushi
Copy link
Owner

I ended up just removing simd-accel altogether because I'm tired of dealing with it. See #2749

@PierreAntoineGuillaume
Copy link
Author

Really nice !

Thanks for your time, and for your tools too.

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 a pull request may close this issue.

2 participants