Baldr: MIPS: Take advantage of guard page to simplify asm.js/wasm memory access

RESOLVED FIXED in Firefox 52

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: hev, Assigned: hev)

Tracking

unspecified
mozilla52
Other
Linux
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
See Bug 1298202
(Assignee)

Comment 1

2 years ago
Created attachment 8792744 [details] [diff] [review]
0001-Bug-1303688-Baldr-MIPS-Take-advantage-of-guard-page-.patch
Attachment #8792744 - Flags: review?(luke)
(Assignee)

Updated

2 years ago
Attachment #8792744 - Flags: review?(luke)
(Assignee)

Comment 2

2 years ago
Created attachment 8792846 [details] [diff] [review]
0001-Bug-1303688-Baldr-MIPS-Take-advantage-of-guard-page-.patch
Attachment #8792744 - Attachment is obsolete: true
Attachment #8792846 - Flags: review?(luke)

Comment 3

2 years ago
Comment on attachment 8792846 [details] [diff] [review]
0001-Bug-1303688-Baldr-MIPS-Take-advantage-of-guard-page-.patch

Review of attachment 8792846 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, and sorry for the churn.

::: js/src/jit/mips-shared/CodeGenerator-mips-shared.cpp
@@ -1779,4 @@
>        default: MOZ_CRASH("unexpected array type");
>      }
>  
> -    memoryBarrier(mir->barrierBefore());

These were removed from the AsmJS* operations because atomics were changed into WasmLoad/Store.  Currently, these two nodes assert there is no barrier, so you'll need to remove those asserts and add the barriers.
Attachment #8792846 - Flags: review?(luke) → review+

Comment 4

2 years ago
Pushed by r@hev.cc:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e7561636a36
Baldr: MIPS: Take advantage of guard page to simplify asm.js/wasm memory access. r=luke
(Assignee)

Comment 5

2 years ago
Created attachment 8793129 [details] [diff] [review]
0002-Bug-1303688-Baldr-MIPS-Move-memory-barrier-to-WasmLo.patch

Comment 6

2 years ago
Pushed by r@hev.cc:
https://hg.mozilla.org/integration/mozilla-inbound/rev/415c88053edd
Baldr: MIPS: Move memory barrier to WasmLoad/Store. r=luke
To wit (for next time): either the second patch should have been folded with the first one, or a bugzilla review should have been requested. Thanks for keeping on updating the MIPS backend, though :)
(Assignee)

Comment 8

2 years ago
(In reply to Benjamin Bouvier [:bbouvier] from comment #7)
> To wit (for next time): either the second patch should have been folded with
> the first one, or a bugzilla review should have been requested. Thanks for
> keeping on updating the MIPS backend, though :)

OK. ;)

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3e7561636a36
https://hg.mozilla.org/mozilla-central/rev/415c88053edd
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.