-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Make some float methods unstable const fn
#130568
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
Conversation
|
The Miri subtree was changed cc @rust-lang/miri Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri |
|
r=me on the interpreter and Miri parts. |
|
I made |
This comment has been minimized.
This comment has been minimized.
You can go ahead and do this in advance, I don't think there is any reason these would be rejected |
This comment has been minimized.
This comment has been minimized.
|
Created tracking issue: #130843 For f16/f32 methods, I placed commented gates under the same feature as f32/f64 (e.g., |
|
☔ The latest upstream changes (presumably #130157) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I believe |
| } | ||
| } | ||
|
|
||
| enum FloatBinOp { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| enum FloatBinOp { | |
| enum FloatBinIntrinsic { |
We use BinOp for the MIR binops, IMO we shouldn't mix up that terminology.
Also please move this above the functions that use it.
|
LGTM from the interpreter side. r? libs-api |
|
Rebased to pick #131256 and remove |
|
☔ The latest upstream changes (presumably #131581) made this pull request unmergeable. Please resolve the merge conflicts. |
|
If libs-api can just okay unstably adding @rustbot label +I-libs-api-nominated |
|
The unstable const APIs LGTM (on the basis that we're doing the same for |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@bors rollup=never |
|
Ah yeah yuck, every function in stdlib that takes a f16 or f128 in the signature still needs |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Some float methods are now `const fn` under the `const_float_methods` feature gate. In order to support `min`, `max`, `abs` and `copysign`, the implementation of some intrinsics had to be moved from Miri to rustc_const_eval.
|
Fixed by adding |
|
@bors r=RalfJung,tgross35 rollup |
Wow f16 / f128 are more unstable than I thought.^^ Is there an issue tracking specifically the codegen crashes? Can we have a central workaround instead of dozens of |
The list of platforms where just seeing the types is enough to make LLVM crash is fortunately pretty limited, wasm and arm64ec being some notably more popular exceptions https://github.com/rust-lang/compiler-builtins/blob/cb060052ab7e4bad408c85d44be7e60096e93e38/configure.rs#L53-L77. AIUI however, we need the inline annotations anyway since Cranelift doesn't support the types, and having anything not inline would mean it can't compile std. I'm not sure what would be better here. |
We can automatically mark functions as |
|
That works 😄 |
|
The codegen crashes are currently tracked as part of the big list in the tracking issue at #116909 (comment), but there isn't a separate dedicated tracking issue just for them. Adding a |
|
Specifically, the logic here, which already makes "small" functions auto- |
Some float methods are now
const fnunder theconst_float_methodsfeature gate.I also made some unstable methods
const fn, keeping their constness under their respective feature gate.In order to support
min,max,absandcopysign, the implementation of some intrinsics had to be moved from Miri to rustc_const_eval (cc @RalfJung).Tracking issue: #130843
r? libs-api
try-job: dist-s390x-linux