758376 - IonMonkey: Differential Testing: Getting 0 vs. NaN with/without ion

Status: RESOLVED DUPLICATE of bug 792234

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase shows different behavior with options --ion -n -m --ion-eager vs. --no-ion on ionmonkey revision c05b873dad48:


function spectralnorm(n) {
  var i, u=[], v=[], w=[], vv=0, vBv=0;
  for (i=0; i<n; ++i) {
    vv  += v[i]*v[i];
  }
  return Math.sqrt(vBv/vv);
}
var actual = '';
for (var i = 6; i <= 48; i *= 2) {
  actual += spectralnorm(i) + ',';
}
print(actual);


$ debug64/js --ion -n -m --ion-eager test.js
NaN,0,0,0,

$ debug64/js --no-ion test.js
NaN,NaN,NaN,NaN,

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

Subtle bug in MDiv folding, if lhs or rhs are constant 0, the other side may be NaN, so this patch just restricts that folding to integer specialization.

Comment 2

[Kannan Vijayan [:djvj]]

Comment on attachment 627426 [details] [diff] [review]
fix

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

::: js/src/ion/MIR.cpp
@@ +687,4 @@
>  
>      // 0 / x -> 0
>      // x / 1 -> x
> +    if (specialization_ == MIRType_Int32 && (IsConstant(lhs(), 0) || IsConstant(rhs(), 1)))

This can be slightly expanded to:

if ((specialization_ == MIRType_Int32 && IsConstant(lhs(), 0)) || IsConstant(rhs(), 1))

As in the rhs() == 1 case, NaN / 1 == Nan == lhs() holds even without the Int32 check.

Comment 3

[Kannan Vijayan [:djvj]]

Missed something:

I don't think the lhs() == 0 case should be included at all, as

0 / I == 0

doesn't hold for all int32 Is, specifically for I=0, as 0/0 == NaN.