Wasm baseline: float.js test failure when run with --no-sse3

RESOLVED FIXED in Firefox 56



JavaScript Engine: JIT
4 months ago
4 months ago


(Reporter: lth, Assigned: bbouvier)


Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 disabled, firefox56 fixed)



(2 attachments)



4 months ago
Created attachment 8877566 [details]
Reduced test case

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...

Comment 1

4 months ago
I guess this blocks tiering, then.
Blocks: 1277562

Comment 2

4 months ago
Looking at these try runs, there are additional failures for --no-sse3.


(I've repro'd the linux results locally on mozilla-inbound c74284491275, my massive patch queue that is underneath those runs is not needed.)

Comment 3

4 months 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: --- → ?

Comment 4

4 months ago
owait, we don't need to track: the wasm baseline compiler isn't enabled there.
tracking-firefox55: ? → ---
tracking-firefox56: ? → ---

Comment 5

4 months ago
Created attachment 8877611 [details] [diff] [review]
Attachment #8877611 - Flags: review?(lhansen)

Comment 6

4 months ago
Comment on attachment 8877611 [details] [diff] [review]

Review of attachment 8877611 [details] [diff] [review]:

Excellent, thank you.
Attachment #8877611 - Flags: review?(lhansen) → review+

Comment 7

4 months ago
Pushed by bbouvier@mozilla.com:
wasm baseline: don't read returned values from x87 FPU stack on x86; r=lth
Last Resolved: 4 months ago
status-firefox56: affected → fixed
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)

Comment 10

4 months 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)
status-firefox54: --- → unaffected
status-firefox55: affected → disabled
status-firefox-esr52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.