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

Make constructors const fn where possible #906

Open
bitshifter opened this issue Sep 14, 2020 · 13 comments
Open

Make constructors const fn where possible #906

bitshifter opened this issue Sep 14, 2020 · 13 comments

Comments

@bitshifter
Copy link
Contributor

I have a library which wraps SIMD types and it would be a nice feature if users were able to create const values with my wrapper types. There's currently no way of doing this in stable Rust.

It would be convenient if some constructor functions could be made const fn so we can create SIMD constants.

For example this is not currently possible:

const ONES : __m128 = unsafe { _mm_set1_ps(1.0) };

Internally functions like _mm_set1_ps are quite straight forward:

pub unsafe fn _mm_set1_ps(a: f32) -> __m128 {
  __m128(a, a, a, a)
}

__m128 is a repr(simd) tuple struct.

I would propose making all of the _mm_set* functions const fn and any other construction functions in stdarch for that matter.

@Amanieu
Copy link
Member

Amanieu commented Sep 14, 2020

cc @rust-lang/libs @rust-lang/wg-const-eval

I think that this functionality can be desirable, however it is an extension to the intrinsics that other compilers don't have. Neither GCC nor Clang allow _mm_set1_ps to be used in const initializers in C (C++ supports dynamic initializers).

@RalfJung
Copy link
Member

These are not really intrinsics currently in the rustc sense, right? __m128 is just the name of the struct type?

Is there a chance that in the future, they might become intrinsics? If not I see little problem with making these functions const.

@Lokathor
Copy link
Contributor

I cannot possibly imagine why, long term, this wouldn't be the completely obvious move for all floating type constructors as well as all integer type constructors and operations. Even in a cross-compile context, the computation can be done at simply a slightly lower speed by doing the work on an array. The only time that a SIMD function shouldn't be const is if we can't be assured of identical results between compile time and execution time.

While it is true that other compilers don't offer this ability, there are a lot of other things they also don't offer, which has never stopped us from adding something to rust. In this specific case, making the functions available as const fn doesn't in any way impact their runtime usage, it's purely a bonus that we would provide to our users compared to alternative SIMD implementations in other languages.

All that said, I think "long term" is key here, because i can imagine all sorts of compiler internal problems as to why we couldn't just mark a function const and get on with the day. Const eval is still fairly early in development.

@RalfJung
Copy link
Member

I don't think const-eval has any specific issues that would be a problem here, beyond what is listed in rust-lang/rust#57241 (but the hard question there is unrelated to const-eval, it is about what our floating-point semantics are in the first place).

@Lokathor
Copy link
Contributor

I was thinking that, for example, to compile time eval an sse2 instruction's usage when you're cross-compiling from an aarch64 device you'd need to have the "runtime code path" where the actual sse2 instruction is inserted, and the "compile time code path" where the sse2 instruction is emulated using an array of [i32;4] values (or whatever values, depending on the operation).

But if we can already do something like that, then that's cool.

@Lokathor
Copy link
Contributor

Lokathor commented Sep 14, 2020

@bitshifter also note that you can make compile time consts of SIMD types using the "const union hack":

https://github.com/Lokathor/wide/blob/main/src/lib.rs#L246

https://github.com/Lokathor/wide/blob/main/src/f32x4_.rs#L15

(Only works in const not const fn)

@RalfJung
Copy link
Member

This here is just about the constructor, not any compiler-supported intrinsic. That's why I asked about intrinsics above.

For SIMD intrinsics, supporting them in CTFE (or Miri) will require an explicit implementation in the Miri engine -- and there's just so many of them that this is a rather dautning task. This implementation is required no matter the host platform; we never use host SIMD (or floating point) for CTFE.

@RalfJung
Copy link
Member

@bitshifter also note that you can make compile time consts of SIMD types using the "const union hack":

You can also use mem::transmute directly these days. ;) (or maybe that is still riding the trains).
Still only in const though.

@bitshifter
Copy link
Contributor Author

I do use the const union hack elsewhere in glam, I just can't use it from a const fn on stable. If the use of mem::transmute from a const fn is stabilised I think that would also work for my use case. I don't know how far off that is though.

@RalfJung
Copy link
Member

RalfJung commented Sep 14, 2020

The current situation (since rust-lang/rust#72920 landed) is that both unions and transmute are stable in const but unstable in const fn. See rust-lang/rust#51909 for the surrounding discussion.

@Lokathor
Copy link
Contributor

The difference is that a const is always evaluated, and then can be checked for validity after the evaluation.

A const fn is not always evaluated, and doesn't have a post-evaluation validity check, so it can potentially have problems.

So that's why transmute is stable in const but not const fn.

@bitshifter
Copy link
Contributor Author

Definitely sounds like making constructors const fn is the simplest change. Does this require a RFC or is it small enough of a change to make a PR?

@RalfJung
Copy link
Member

For marking them unstably const, a PR is fine I think.

The tracking issue should point out that stabilization is blocked on rust-lang/rust#57241.

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

No branches or pull requests

4 participants