Closed
Bug 1316385
Opened 6 years ago
Closed 6 years ago
wasm: generate bounds check the same way in debug/opt builds
Categories
(Core :: JavaScript Engine: JIT, defect, P3)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(1 file)
11.17 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
Bug 1315596 wasn't triggering under a debug build because a redundant check was generated anyways and this perturbed the register allocation patterns. I think using debug vs opt mode shouldn't have an impact here; in particular, this might hide a few fuzz bugs. Here's a patch offering a new option for the shell (--wasm-check-bce) as well as the env option (JITOPTIONS_wasmAlwaysCheckBounds) to trigger the old behavior. It's enabled on the wasm/memory and wasm/bce tests; but really we could use it for all the tests by including it in the directives.txt file (thanks Lars!).
Attachment #8809096 -
Flags: review?(luke)
![]() |
||
Comment 1•6 years ago
|
||
Comment on attachment 8809096 [details] [diff] [review] wasm-check-bce.patch Review of attachment 8809096 [details] [diff] [review]: ----------------------------------------------------------------- Nice! ::: js/src/jit-test/jit_test.py @@ +200,5 @@ > # wasm-baseline run when requesting --jitflags=interp, but the test > # contains test-also-noasmjs.) > test_flags = get_jitflags(options.jitflags) > options.can_test_also_noasmjs = True > + options.can_test_also_wasm = True Perhaps instead we could name 'asmjs_disabled'/'wasm_disabled'?
Attachment #8809096 -
Flags: review?(luke) → review+
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/07bbb0e3dde4 Add an env option to always enable wasm bounds check generation; r=luke
Updated•6 years ago
|
Priority: -- → P3
Comment 3•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/07bbb0e3dde4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•