Closed
Bug 1448329
Opened 7 years ago
Closed 7 years ago
Consider removing static typed array accesses
Categories
(Core :: JavaScript Engine: JIT, enhancement, P3)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla61
People
(Reporter: jandem, Assigned: jandem)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
|
(deleted),
patch
|
tcampbell
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
MLoadTypedArrayElementStatic and MStoreTypedArrayElementStatic are only used on 32-bit x86 and only for asm.js-like code that uses singleton arrays.
Removing these instructions also means we don't have to worry about Spectre mitigations for them.
|
Assignee | |
Comment 1•7 years ago
|
||
Attachment #8962289 -
Flags: review?(nicolas.b.pierron)
|
|
Assignee | |
Comment 2•7 years ago
|
||
Comment on attachment 8962289 [details] [diff] [review]
Patch
nbp is at the Rust all hands this week.
Attachment #8962289 -
Flags: review?(nicolas.b.pierron) → review?(tcampbell)
|
||
Comment 3•7 years ago
|
||
Comment on attachment 8962289 [details] [diff] [review]
Patch
Review of attachment 8962289 [details] [diff] [review]:
-----------------------------------------------------------------
That's a nice diffstat! Argument for removal sounds reasonable.
Attachment #8962289 -
Flags: review?(tcampbell) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9db236e8d1de
Remove 32-bit-x86-only static typed array access optimization. r=tcampbell
|
||
Updated•7 years ago
|
Priority: -- → P3
|
||
Comment 5•7 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
|
|
Assignee | |
Comment 6•7 years ago
|
||
Comment on attachment 8962289 [details] [diff] [review]
Patch
Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: We want to uplift Spectre fixes in 61 to 60.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just removes a 32-bit-only optimization.
[String changes made/needed]: None.
Attachment #8962289 -
Flags: approval-mozilla-beta?
|
||
Comment 7•7 years ago
|
||
Comment on attachment 8962289 [details] [diff] [review]
Patch
Spectre mitigation needed for 60. Approved for 60.0b11.
Attachment #8962289 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
|
|
Assignee | |
Comment 9•7 years ago
|
||
| uplift | ||
status-firefox60:
--- → fixed
Flags: needinfo?(jdemooij)
You need to log in
before you can comment on or make changes to this bug.
Description
•