1379626 - Naïve ToInteger implementation can lead to repeated bailouts in Math.ceil

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine: JIT, enhancement, P3)

Description

[André Bargull [:anba]]

Test case:
---
function Number_isInteger(number) {
    // Step 1.
    if (typeof number !== "number")
        return false;

    // Step 3.
    var integer = number >= 0 ? Math.floor(number) : Math.ceil(number);

    // Steps 2, 4.
    if (integer - number !== 0)
        return false;

    // Step 5.
    return true;
}
Number.isInteger = Number_isInteger;

// Values are non-int32 and INT32_MIN.
var values = [4.2, -(2**31)];

var t = Date.now();
for (var i = 0; i < 10000000; ++i) {
    Number.isInteger(values[i % values.length]);
}
print(Date.now() - t);
---


The test case leads to repeated bailouts, IONFLAGS=bailouts shows "[IonBailouts]  bailing from bytecode: call, MIR: ceil [14], LIR: ceil [13]".

Comment 1

[Jan de Mooij [:jandem]]

Probably because of the bailoutCvttsd2si in CodeGeneratorX86Shared::visitCeil. We bail because we get INT32_MIN which happens to be the value the CPU throws at us for "not an integer valued double" :(

Comment 2

[André Bargull [:anba]]

Warp prevents repeated bailouts from happening for this test case. The bailoutCvttsd2si issue noted in comment #1 is still present, e.g. when var values = [-4.2, -(2**31)]; is used, so both values are negative, Warp will stop inlining Math.ceil because the fallback stub was taken. If that issue ever becomes are problem in practice, we should track it under a different bug.