911301 - IonMonkey: prevent Float32 deoptimization due to OSR

Status: RESOLVED DUPLICATE of bug 888109

(Core :: JavaScript Engine, defect)

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

Test framework in bug 888109 shows that if OSR are activated, Float32 specialization can't take place in certain situations. For instance:

function() {
  var x = f32[0];
  while(true) {
    x = Math.fround(x + 1);
  }
  f32[0] = x;
}

In the regular non-OSR path, x has the float32 mirtype. But in the OSR block, as there is no notion of jsvalue's type float32, the OSR value has a double type. As a matter of fact, the phi (joining the first block and the OSR block values of x) at the loop header can't be a float32 producer, which prevents float32 specialization for the addition.

This could be prevented by pattern-matching the inputs of phis: if one of them is an OSR value (or a type guard on an OSR value) with double type, we could mark it as being a potential producer too.

A slightly different solution could be to say that:
- a type guarded value can be a producer iff its input is a producer
- an OSR value can be a producer iff it contains a double.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

After a few hours of debugging, it shows up that the problem wasn't OSR but --ion-eager in the example test case. I will update the test harness patch in bug 888109 so that the assertFloat32 doesn't blow up everything with --ion-eager.

Also, the case I was seeing is that |x+1| is seen as an addition between a Float32 and an Integer during Ion building. During the call to infer, no direct way to specialize is found and thus the infer fallback function is called. This one looks at the observed types in baseline and was concluding that this op can be a Double operation. Later, during type analysis, this operation could be specialized as Float32.

In some cases, the infer fallback won't be able to specialize the operation. The solution is to say that if one of the two operands is a Float32, we specialize the operation as a Double operation. This way, the ApplyTypes pass can decide whether or not to specialize the operation. As it's a one line fix, I directly put it in the MIR patch in bug 888109.