-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Improve SIMD casts #92425
Improve SIMD casts #92425
Conversation
r? @estebank (rust-highfive has picked a reviewer for you, use r? to override) |
☔ The latest upstream changes (presumably #92518) made this pull request unmergeable. Please resolve the merge conflicts. |
3c5c375
to
3cd4976
Compare
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.
Brain finally thawed and allowed me to review things. Thank you for your patience. Only minimal remarks.
assert!(matches!(self.cx().type_kind(float_ty), TypeKind::Float | TypeKind::Double)); | ||
assert_eq!(self.cx().type_kind(int_ty), TypeKind::Integer); |
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.
Hm, isn't this redundant with the checks performed by calling float_width
and then matching on that? I suppose it doesn't exactly hurt to try to catch it in a single place here but...
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.
Yeah, it should error there, but I figure it's good to ensure that everything is the way we expect before going on.
Co-authored-by: Jubilee <46493976+workingjubilee@users.noreply.github.com>
That's fine. It will probably be more useful with a specific diagnostic message, but probably an ICE will eventually hit that path and inform what that message should have been. With that, everything looks like it should be in order! And in theory this is entirely additive so, @bors r+ rollup=always |
📌 Commit 49d36d7 has been approved by |
…askrgr Rollup of 9 pull requests Successful merges: - rust-lang#90782 (Implement raw-dylib support for windows-gnu) - rust-lang#91150 (Let qpath contain NtTy: `<$:ty as $:ty>::…`) - rust-lang#92425 (Improve SIMD casts) - rust-lang#92692 (Simplify and unify rustdoc sidebar styles) - rust-lang#92780 (Directly use ConstValue for single literals in blocks) - rust-lang#92924 (Delete pretty printer tracing) - rust-lang#93018 (Remove some unused `Ord` derives based on `Span`) - rust-lang#93026 (fix typo in `max` description for f32/f64) - rust-lang#93035 (Fix stdarch submodule pointing to commit outside tree) Failed merges: - rust-lang#92861 (Rustdoc mobile: put out-of-band info on its own line) r? `@ghost` `@rustbot` modify labels: rollup
simd_cast
intrinsic to takeusize
andisize
simd_as
intrinsic, which is the same assimd_cast
except for saturating float-to-int conversions (matching the behavior ofas
).cc @workingjubilee