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

Switch to stable Rust toolchain #592

Open
3 tasks done
rinon opened this issue Nov 29, 2023 · 14 comments
Open
3 tasks done

Switch to stable Rust toolchain #592

rinon opened this issue Nov 29, 2023 · 14 comments
Assignees
Labels
low priority Issues that we would like to address at some point in the future

Comments

@rinon
Copy link
Collaborator

rinon commented Nov 29, 2023

Ideally rav1d should build with a stable toolchain at some point. It looks like we ended up with a few nightly-only features from the initial c2rust transpile that we should eventually remove. I currently find three nightly-only features that we use:

The intrinsics we can take care of by using stabilized Rust atomic types, the variadic usage appears to just be vfprintf which would be easy to remove, and for extern types we can probably do opaque pointers in another way.

@rinon rinon added the low priority Issues that we would like to address at some point in the future label Nov 29, 2023
@kkysen
Copy link
Collaborator

kkysen commented Nov 29, 2023

The extern types might be all gone now after we deduplicated everything. At least the rav1d library; the few binaries still have a bunch that are hard to unravel due to type erasure generics.

@kkysen
Copy link
Collaborator

kkysen commented Nov 29, 2023

I've been meaning to fix all the atomic intrinsics to use atomic types soon; that'll be part of making the core data types safe.

@kkysen
Copy link
Collaborator

kkysen commented Nov 29, 2023

For the variadics, I'm not sure we can fully get rid of those because there are callbacks the user can set and we need to keep that ABI. Eventually we can put it behind a legacy C API feature flag, as we shouldn't need it for pure Rust usage.

@rinon
Copy link
Collaborator Author

rinon commented Nov 29, 2023

The extern types might be all gone now after we deduplicated everything. At least the rav1d library; the few binaries still have a bunch that are hard to unravel due to type erasure generics.

Could we do the same thing as bindgen and make the type unconstructable? We don't care what it's purported layout is, only that we can't construct it.

I've been meaning to fix all the atomic intrinsics to use atomic types soon; that'll be part of making the core data types safe.

Do we have an issue on this? I was just looking at that, will simplify some things nicely.

For the variadics, I'm not sure we can fully get rid of those because there are callbacks the user can set and we need to keep that ABI. Eventually we can put it behind a legacy C API feature flag, as we shouldn't need it for pure Rust usage.

Eek ok, yeah. This whole issue is super low priority, I just wanted to create an issue on it while I was thinking about it.

@kkysen
Copy link
Collaborator

kkysen commented Nov 29, 2023

Could we do the same thing as bindgen and make the type unconstructable? We don't care what it's purported layout is, only that we can't construct it.

I'm not sure, but the binaries I don't think are very important compared to the rav1d library that others will use. And there are Rust libraries we can use for the demuxing in those binaries instead if we want to make them safe, sidestepping the issue. But I don't think people depend on these binaries (except our tests I guess), so it's a very low priority.

I've been meaning to fix all the atomic intrinsics to use atomic types soon; that'll be part of making the core data types safe.

Do we have an issue on this? I was just looking at that, will simplify some things nicely.

No, I don't think so. Feel free to open one.

Eek ok, yeah. This whole issue is super low priority, I just wanted to create an issue on it while I was thinking about it.

👍

@thedataking
Copy link
Collaborator

But I don't think people depend on these binaries (except our tests I guess), so it's a very low priority.

Exactly. If the Rust binaries give us any trouble, I think we can delete them and build the C sources with meson. I have a patch somewhere to do that already – that's one way to test that we remain C compatible.

@kkysen
Copy link
Collaborator

kkysen commented Nov 29, 2023

Exactly. If the Rust binaries give us any trouble, I think we can delete them and build the C sources with meson. I have a patch somewhere to do that already – that's one way to test that we remain C compatible.

I'm not sure if we should fully delete them in case someone finds them useful for a rust version of them, but I'd be in favor of testing with the C versions of them (though it does complicate the build system, unfortunately).

rinon added a commit that referenced this issue Dec 2, 2023
This removes #![feature(extern_types)] just for the `rav1d` library,
which is what really matters (unlike the binaries).

All types have been deduplicated now, so we don't need this anymore.

Fixes part of #592.
@kkysen
Copy link
Collaborator

kkysen commented Feb 5, 2024

With #688 fixing #638, we can remove #![feature(core_intrinsics)] now.

kkysen added a commit that referenced this issue Feb 16, 2024
* Fixes part of #592

Feature no longer needed now that we use stable SIMD APIs.
@negge
Copy link

negge commented Feb 29, 2024

Is the minimum rust version documented anywhere in the project? I could not get rav1d main to compile with 1.76.0-nightly on aarch64.

@kkysen
Copy link
Collaborator

kkysen commented Feb 29, 2024

Is the minimum rust version documented anywhere in the project? I could not get rav1d main to compile with 1.76.0-nightly on aarch64.

Hi! Yes, we have our nightly pinned in rust-toolchain.toml. cargo should use it automatically. How are you trying to build rav1d?

We can remove the last nightly feature, but it wasn't a priority, so we haven't done it quite yet. Once we do, we'll switch to stable.

@kkysen kkysen closed this as completed Feb 29, 2024
@kkysen kkysen reopened this Feb 29, 2024
@negge
Copy link

negge commented Feb 29, 2024

Hi! Yes, we have our nightly pinned in rust-toolchain.toml. cargo should use it automatically. How are you trying to build rav1d?

I filed issue #773 with the exact build command. Just a simple cargo build --release.

@kkysen
Copy link
Collaborator

kkysen commented May 23, 2024

With #620, we'll be able to build rav1d on stable, with the following exceptions:

For #![feature(stdarch_arm_feature_detection)] and #![feature(stdarch_riscv_feature_detection)], I'm not sure if those will be stabilized soon, but it still seems better to do it with this officially supported way than the dav1d way of manually detecting CPU features.

It would be nice if we can get rust_toolchain.toml to use channel = "stable" only for rav1d and for x86, x86_64, and aarch64.

kkysen added a commit that referenced this issue Jun 25, 2024
Remove by replacing the remaining use with a `mem::transmute` from a
non-variadic `fn` to a variadic `fn`, as we don't actually use the
variadic args, we just need its type.

* Fixes part of #592.
@rinon
Copy link
Collaborator Author

rinon commented Jul 10, 2024

I'm gonna do a quick fix up on the tools so we can be on stable.

@rinon rinon self-assigned this Jul 10, 2024
@kkysen
Copy link
Collaborator

kkysen commented Jul 10, 2024

I'm gonna do a quick fix up on the tools so we can be on stable.

I'm not sure it's so simple to get rid of #![feature(extern_types)] since the types are different in different modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low priority Issues that we would like to address at some point in the future
Projects
None yet
Development

No branches or pull requests

4 participants