Closed
Bug 1372883
Opened 7 years ago
Closed 7 years ago
Wasm baseline: float.js test failure when run with --no-sse3
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | disabled |
firefox56 | --- | fixed |
People
(Reporter: lth, Assigned: bbouvier)
References
Details
Attachments
(2 files)
(deleted),
application/x-javascript
|
Details | |
(deleted),
patch
|
lth
:
review+
|
Details | Diff | Splinter Review |
The f32.floor test in wasm/float.js computes a NaN rather than 40 as it should when the baseline compiler is run with --no-sse3. (Other tests in that file may fail too in this configuration, haven't checked yet.) This is particularly odd, as the baseline compiler generates a callout for floor always; ABI problem?
Only a problem on x86, ie, 32-bit. Looks like it is both optimized and non-optimized builds, both DEBUG and not.
Also happens on 32-bit Windows 7 runs.
Locally I'm compiling with -mfpmath=sse -msse -msse2, but on try (where I first saw this) I'm using whatever flags are the default, so I don't think that's it.
Beats me why we haven't seen this before, maybe --wasm-always-baseline is not used as extensively as we think it should be? Tiering will take care of that...
Reporter | ||
Comment 2•7 years ago
|
||
Looking at these try runs, there are additional failures for --no-sse3.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1a3573aa167d777787c6de97b2acffd5a18b532&selectedJob=106943751
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1a3573aa167d777787c6de97b2acffd5a18b532&selectedJob=106954494
(I've repro'd the linux results locally on mozilla-inbound c74284491275, my massive patch queue that is underneath those runs is not needed.)
Assignee | ||
Comment 3•7 years ago
|
||
[Tracking Requested - why for this release]: correctness issues in wasm for x86 machines *without* SSE4 support.
Bug 1340219 changed the ABI: return paths of floating-point functions don't need to do the x87 FPU dance anymore, under wasm ABI.
Assignee: lhansen → bbouvier
Blocks: 1340219
status-firefox55:
--- → affected
status-firefox56:
--- → affected
tracking-firefox55:
--- → ?
tracking-firefox56:
--- → ?
Assignee | ||
Comment 4•7 years ago
|
||
owait, we don't need to track: the wasm baseline compiler isn't enabled there.
tracking-firefox55:
? → ---
tracking-firefox56:
? → ---
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8877611 -
Flags: review?(lhansen)
Reporter | ||
Comment 6•7 years ago
|
||
Comment on attachment 8877611 [details] [diff] [review]
fix.patch
Review of attachment 8877611 [details] [diff] [review]:
-----------------------------------------------------------------
Excellent, thank you.
Attachment #8877611 -
Flags: review?(lhansen) → review+
Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/31758734ff3c
wasm baseline: don't read returned values from x87 FPU stack on x86; r=lth
Comment 8•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 9•7 years ago
|
||
Did you want to request beta uplift on this one, or is this wontfix for 55 because baseline is unused?
Flags: needinfo?(bbouvier)
Assignee | ||
Comment 10•7 years ago
|
||
The baseline compiler isn't enabled by default in 55 (can be enabled through a pref though). I don't think there's any intent to uplift a patch that would enable the wasm baseline compiler. So we don't need to uplift this patch. Thanks for the heads-up!
Flags: needinfo?(bbouvier)
Updated•7 years ago
|
status-firefox54:
--- → unaffected
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•