Problem with Typed Float32 Arrays and canonicalizeNaNs() / LIR_cmovd

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: humph, Assigned: gal)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(blocking2.0 beta4+)

Details

(Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin)

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
In bug 490705 I have a patch that uses typed float32 arrays, and bug Bug 584158 has made it so that they are mostly filled with zeros or NaNs.  Before I would see:

-9.063918018914485e-12,-3.261597155551632e-12,-7.691810383070319e-11,-4.440857404031107e-12,-1.680772881984538e-10,6.000556301843929e-11,-1.9150961372282893e-10,2.4246665786265e-10,-4.565171851655947e-11,5.450795370620654e-10,3.362514988669574e-10,9.004306744664348e-10,9.466241124300723e-10,1.175186170598863e-9,1.6831636084901902e-9,1.1990984871701471e-9,2.3562511941577213e-9,8.144737662085788e-10,2.725613068577104e-9,-6.554523590551753e-11,2.5597279851297117e-9,-1.405781158148045e-9,1.7045438394092116e-9,-3.0212734536405605e-, ...

and now I get:

-9.063918018914485e-12,-3.261597155551632e-12,-6.2774385622041925e+66,-2.4983353906949635e-127,18563148,18563148,0,0,0,0,0,0,0,0,-2.4983353906949635e-127,18563724,18563724,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,-2.4983353906949635e-127,18564300,18564300,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,-2.4983353906949635e-127,18564876,18564876,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,

Bisecting builds I see that http://hg.mozilla.org/mozilla-central/rev/48822#l1.23 is to blame.  If I revert this, it works again:

    1.23 -        v_ins = lir->insLoad(LIR_ldf2d, addr_ins, 0, ACCSET_OTHER);
    1.24 +        v_ins = canonicalizeNaNs(lir->insLoad(LIR_ldf2d, addr_ins, 0, ACCSET_OTHER));

Comment 1

7 years ago
cmovd bug?
Summary: Problem with Typed Float32 Arrays and canonicalizeNaNs() → Problem with Typed Float32 Arrays and canonicalizeNaNs() / LIR_cmovd
(Assignee)

Comment 2

7 years ago
Lets make a shell test case.
var v = new Float32Array(32);

for (var i = 0; i < v.length; ++i)
  v[i] = i;

var t = 0;
for (var i = 0; i < v.length; ++i)
  t += v[i];

print(t);

vladimir@lightning[1651]$ ./js x.js
496
vladimir@lightning[1652]$ ./js -j x.js
4
Created attachment 464969 [details]
TMFLAGS=full output
(Assignee)

Comment 5

7 years ago
Created attachment 464991 [details] [diff] [review]
patch
Assignee: general → gal
(Assignee)

Comment 6

7 years ago
If the condition included one of the two operands of the cmov, asm_branch could change the register allocation of iftrue on the fly. That caused bad code to be emitted.
(Assignee)

Updated

7 years ago
Attachment #464991 - Flags: review?(nnethercote)
(Assignee)

Updated

7 years ago
blocking2.0: --- → ?

Updated

7 years ago
blocking2.0: ? → beta4+
(Assignee)

Updated

7 years ago
Attachment #464991 - Flags: review?(edwsmith)
Attachment #464991 - Flags: review?(nnethercote) → review+
(Reporter)

Comment 7

7 years ago
I've just tested and this patch fixes my issue, thanks for doing it so quickly.

Updated

7 years ago
Attachment #464991 - Flags: review?(edwsmith) → review+
http://hg.mozilla.org/mozilla-central/rev/6b2ac4552e1a

(m-c, not on tm)
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Vlad, NJ changes need to land on nanojit-central first, else they'll be clobbered by NJ-to-TM merges.  See https://developer.mozilla.org/en/NanojitMerge.  I'm sure you already knew this and just temporarily forgot about it :)

http://hg.mozilla.org/projects/nanojit-central/rev/2e44b58e0662
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: fixed-in-nanojit
http://hg.mozilla.org/tracemonkey/rev/b8e76c9b23d1
Whiteboard: fixed-in-nanojit → fixed-in-nanojit, fixed-in-tracemonkey

Comment 11

7 years ago
http://hg.mozilla.org/mozilla-central/rev/b8e76c9b23d1
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED

Comment 12

7 years ago
http://hg.mozilla.org/tamarin-redux/rev/2f2841596809
Whiteboard: fixed-in-nanojit, fixed-in-tracemonkey → fixed-in-nanojit, fixed-in-tracemonkey, fixed-in-tamarin
You need to log in before you can comment on or make changes to this bug.