586387 - Problem with Typed Float32 Arrays and canonicalizeNaNs() / LIR_cmovd

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Humphrey (:humph)]

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

[Luke Wagner [:luke]]

cmovd bug?

Comment 2

[Andreas Gal :gal]

Lets make a shell test case.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: TMFLAGS=full output

Comment 5

[Andreas Gal :gal]

Attachment: patch

Comment 6

[Andreas Gal :gal]

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.

Comment 7

[David Humphrey (:humph)]

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

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

http://hg.mozilla.org/mozilla-central/rev/6b2ac4552e1a

(m-c, not on tm)

Comment 9

[Nicholas Nethercote [inactive]]

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

Comment 10

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/b8e76c9b23d1

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/b8e76c9b23d1

Comment 12

[Steven Johnson]

http://hg.mozilla.org/tamarin-redux/rev/2f2841596809