983448 - masm.convertUInt32ToFloat32 gives wrong results on x86

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Luke Wagner [:luke]]

Attachment: fix-unsigned-to-float32

convertUInt32ToFloat32 uses the same scheme as convertUInt32ToDouble (subtract 2^31, cvtsi2sd, add 2^31), except that this doesn't work for float because float can't precisely represent integers as large as 2^31 so the final addition ends up producing the wrong number for small integers.  (Atm, convertUint3ToFloat32 is only generated from MAsmJSUnsignedToFloat32 so this is an asm.js-only bug.)

The C++ compiler does various arcane things for unsigned-to-float conversion but, seeing as this is a fairly rare operation, I just went with
  convertUint32ToDouble
  convertDoubleToFloat32
in this patch.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8390885 [details] [diff] [review]
fix-unsigned-to-float32

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

Doh, nice catch!

Comment 2

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cace5473227e

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cace5473227e

Comment 4

[Luke Wagner [:luke]]

Comment on attachment 8390885 [details] [diff] [review]
fix-unsigned-to-float32

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 904918
User impact if declined: incorrect JS results
Testing completed (on m-c, etc.): m-c, m-a
Risk to taking this patch (and alternatives if risky): very low

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/87251be46cc7

Comment 6

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

https://hg.mozilla.org/releases/mozilla-beta/diff/87251be46cc7/js/src/jit-test/tests/asm.js/testFloat32.js