Closed Bug 1115742 Opened 5 years ago Closed 5 years ago

SIMD in Odin: don't canonicalize NaN on SimdExtractElement

Categories

(Core :: JavaScript Engine: JIT, enhancement)

x86_64
All
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: sunfish, Assigned: sunfish)

Details

Attachments

(1 file)

NaN canonicalization turns out to be a significant overhead in https://github.com/chadaustin/Web-Benchmarks/tree/master/skinning . Alon reminded me that for asm.js we can omit NaN canonicalization code.

The attached patch alone delivers about a 30% speedup on that benchmark. We may be seeing the effects of "port 5" pressure on Sandy Bridge, since the hot loop contains numerous shuffle operations, and the NaN canonicalization sequence uses a branch.

While this patch is enough for asm.js, in non-asm.js mode we may wish to consider an optimization to eliminate NaN canonicalizations when it's safe for all users of a value. In the above benchmark, the canonicalized value is immediately stored to a typed array, so it seems like it ought to be possible to optimize it away.
Attachment #8541741 - Flags: review?(luke)
Comment on attachment 8541741 [details] [diff] [review]
asmjs-no-canonicalize.patch

I agree that it seems like we could perform an analysis to determine that all uses of a float/double def are SIMD and other non-canonical-NaN-safe operations and then canonicalize all floats/doubles on bail.
Attachment #8541741 - Flags: review?(luke) → review+
Is it fine to have non canonical NaNs flow into the interpreter / baseline / ion (not only float NaNs but also double NaNs)? If not, this would be observable when reading a lane from a float32x4, converting it to double and returning it to non-asmjs code (just checked, cvtss2sd doesn't canonicalize NaN inputs).
(In reply to Benjamin Bouvier [:bbouvier] from comment #2)
> Is it fine to have non canonical NaNs flow into the interpreter / baseline /
> ion (not only float NaNs but also double NaNs)? If not, this would be
> observable when reading a lane from a float32x4, converting it to double and
> returning it to non-asmjs code (just checked, cvtss2sd doesn't canonicalize
> NaN inputs).

Odin canonicalizes NaNs when they get returned from functions, so non-canonical NaNs don't escape.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0bc70a92b047
https://hg.mozilla.org/mozilla-central/rev/0bc70a92b047
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.