Drop handrolled NEON SIMD wrappers.
Categories
(Core :: Graphics: Color Management, defect)
Tracking
()
People
(Reporter: jrmuizel, Assigned: jrmuizel)
References
Details
Attachments
(1 file)
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
We require Rust 1.50 now which is new enough to not need these
wrappers anymore. (edit: This comment was wrong at the time of writing. We required 1.47 then)
Assignee | ||
Comment 1•4 years ago
|
||
We require Rust 1.50 now which is new enough to not need these
wrappers anymore.
Comment 3•4 years ago
|
||
bugherder |
I do get this beauty when compiling firefox-88.0_beta3 for arm:
2:00.25 error[E0432]: unresolved imports `core::arch::arm::vcvtq_s32_f32`, `core::arch::arm::vgetq_lane_s32`, `core::arch::arm::vld1q_dup_f32`, `core::arch::arm::vld1q_f32`, `core::arch::arm::vmaxq_f32`, `core::arch::arm::vminq_f32`
2:00.25 --> gfx/qcms/src/transform_neon.rs:9:40
2:00.25 |
2:00.25 9 | float32x4_t, int32x4_t, vaddq_f32, vcvtq_s32_f32, vgetq_lane_s32, vld1q_dup_f32, vld1q_f32,
2:00.25 | ^^^^^^^^^^^^^ ^^^^^^^^^^^^^^ ^^^^^^^^^^^^^ ^^^^^^^^^ no `vld1q_f32` in `arch::arm`
2:00.25 | | | |
2:00.25 | | | no `vld1q_dup_f32` in `arch::arm`
2:00.25 | | no `vgetq_lane_s32` in `arch::arm`
2:00.25 | no `vcvtq_s32_f32` in `arch::arm`
2:00.25 10 | vmaxq_f32, vminq_f32, vmulq_f32,
2:00.25 | ^^^^^^^^^ ^^^^^^^^^ no `vminq_f32` in `arch::arm`
2:00.25 | |
--
2:00.26 | ^^^^^^^^^
2:00.26 help: a similar name exists in the module
2:00.26 |
2:00.26 10 | vmaxq_f32, vmvnq_s32, vmulq_f32,
2:00.26 | ^^^^^^^^^
I came here through the gecko-dev blame function - does your patch here raise minimal rust to rust-1.50.0 then? If so, please confirm so I can upgrade the toolchain - and please bump the config scripts accordingly to ask for propper rust version. Thank you :)
Assignee | ||
Comment 5•4 years ago
•
|
||
Ah, I thought it had been bumped. We should probably just revert this then. No need to raise it unnecessarily.
Assignee | ||
Updated•4 years ago
|
Comment 6•4 years ago
|
||
backout |
Backed out as requested.
Backout link: https://hg.mozilla.org/integration/autoland/rev/8967f5b384d10afe1e0b89b38546e45cbc1da0c6
Comment 7•4 years ago
|
||
backout |
Backed out from Beta for 88.0b6 also.
https://hg.mozilla.org/releases/mozilla-beta/rev/63e89bee687e
thanks for the backout, just wanted to tell you that there is nothing wrong with the code, as it compiles fine with rust-1.50.0
Comment 9•4 years ago
|
||
Backout merged: https://hg.mozilla.org/mozilla-central/rev/8967f5b384d1
Comment 10•4 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jrmuizel, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 11•4 years ago
|
||
This needs to wait for MSRV bump
Comment 12•3 years ago
|
||
Comment 13•3 years ago
|
||
bugherder |
Assignee | ||
Comment 15•3 years ago
|
||
Probably wouldn't hurt as it should help avoid breakage with newer rust compilers.
Assignee | ||
Comment 16•3 years ago
|
||
Comment on attachment 9208933 [details]
Bug 1697818 - Drop handrolled NEON SIMD wrappers.
Beta/Release Uplift Approval Request
- User impact if declined: Firefox is less likely to compile with newer rust compilers because of its use of unstable rust features
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The only risk of this is that it breaks someone's build
- String changes made/needed:
Comment 17•3 years ago
|
||
Comment on attachment 9208933 [details]
Bug 1697818 - Drop handrolled NEON SIMD wrappers.
Approved for 91 beta 6, thanks.
Comment 18•3 years ago
|
||
bugherder uplift |
Description
•