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)

x86
All
defect
Not set
normal

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)

Attached file Reduced test case (deleted) —
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...
I guess this blocks tiering, then.
Blocks: 1277562
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.)
[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
owait, we don't need to track: the wasm baseline compiler isn't enabled there.
Attached patch fix.patch (deleted) — Splinter Review
Attachment #8877611 - Flags: review?(lhansen)
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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Did you want to request beta uplift on this one, or is this wontfix for 55 because baseline is unused?
Flags: needinfo?(bbouvier)
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)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: