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

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Lars T Hansen [:lth]]

Attachment: 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

[Lars T Hansen [:lth]]

I guess this blocks tiering, then.

Comment 2

[Lars T Hansen [:lth]]

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

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

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

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

owait, we don't need to track: the wasm baseline compiler isn't enabled there.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: fix.patch

Comment 6

[Lars T Hansen [:lth]]

Comment on attachment 8877611 [details] [diff] [review]
fix.patch

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

Excellent, thank you.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
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

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/31758734ff3c

Comment 9

[Julien Cristau [:jcristau]]

Did you want to request beta uplift on this one, or is this wontfix for 55 because baseline is unused?

Comment 10

[Benjamin Bouvier [:bbouvier] (inactive)]

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!