946679 - Differential Testing: Ion incorrectly gets NaN with max of fround

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jesse Ruderman]

function f(x) {
    return Math.max(Math.fround(0), Math.fround(Math.exp(x)));
}
f(0);
f(0);
print(f(1));

Without --ion-eager: 2.7182817459106445
With --ion-eager: NaN

IIRC, autobisect pointed at:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a39a26962730
user:        Sean Stangl
date:        Thu Nov 07 15:19:55 2013 -0800
summary:     Bug 936234 - Inline n-Ary Math.min and Math.max. r=shu

Comment 1

[Sean Stangl [:sstangl]]

The problem isn't with Math.max: the problem is only reproducible with that patch because it changed the logic to prevent inlining bad situations involving floats.

It looks like someone recently made MPassArg have a return type/typeset. The problem is that these values are not being kept in sync with those of the argument after initialization -- rejecting the inlining step then causes the MPassArg to still have type Float32 even though the MPassArg has a NoFloatPolicy, and we perform garbage.

The right way forward is probably to reinstate the previous behavior, where MPassArg was a transparent wrapper with result type MIRType_None. Jan, do you know why this change to MPassArg was made?

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Sean Stangl [:sstangl] from comment #1)
> The right way forward is probably to reinstate the previous behavior, where
> MPassArg was a transparent wrapper with result type MIRType_None. Jan, do
> you know why this change to MPassArg was made?

Are you sure? According to |hg annotate| bug 787343 changed it from MIRType_Value to the type of the input, but that was more than a year ago..

Comment 3

[Sean Stangl [:sstangl]]

Ahem, yes, I meant MIRType_Value. I modified LIRGenerator::visitPassArg() to assert that the type of the MPassArg is the same as that of its argument, and the assertion blows up a number of jit-tests.

Comment 4

[Sean Stangl [:sstangl]]

This was fixed by the removal of MPassArg.

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Sean Stangl [:sstangl] from comment #4)
> This was fixed by the removal of MPassArg.

FIXED by bug 952992 then. Thanks!