Skip to content

Commit

Permalink
Cherry-pick 270481@main (79a659d). https://bugs.webkit.org/show_bug.c…
Browse files Browse the repository at this point in the history
…gi?id=264278

    [JSC] Fix addImmediateShouldSpeculateInt32 for case int32 + constant double
    https://bugs.webkit.org/show_bug.cgi?id=264278
    rdar://117563215

    Reviewed by Yusuke Suzuki.

    Current fixup phase would convert the DFG node

        ValueAdd(Int32, DoubleConstant)

    to

        ArithAdd(Int32, JSConstant(DoubleConstant))

    if the node is OK to be truncated to Int32. This is
    wrong. For example, let Int32 be -1 and DoubleConstant
    be 0.1 where

        ToInt32(-1 + 0.1) == 0

    but

        -1 + ToInt32(0.1) == -1

    . So, we should not speculate that node with Int32 in this case.

    * JSTests/stress/dfg-int32-add-non-int-double.js: Added.
    (foo):
    * Source/JavaScriptCore/dfg/DFGGraph.h:

    Canonical link: https://commits.webkit.org/270481@main
  • Loading branch information
hyjorc1 authored and aperezdc committed Jan 25, 2024
1 parent 958bda0 commit 60a14f2
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
18 changes: 18 additions & 0 deletions JSTests/stress/dfg-add-int32-var-with-const-double.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
let expected = 0;

function foo(arg) {
let x = /\s/;
const a = 0 | arg;
const b = a + 0.1;
const c = b >> x;
if (expected != 0 && c != expected)
throw new Error("bad c " + c + " expected " + expected);
return c;
}
noInline(foo);

let y = -2147483647;
expected = foo(y);
for (let i = 0; i < 1e4; i++) {
foo(y);
}
12 changes: 11 additions & 1 deletion Source/JavaScriptCore/dfg/DFGGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -1328,7 +1328,17 @@ class Graph final : public virtual Scannable {
if (doubleImmediate < -twoToThe48 || doubleImmediate > twoToThe48)
return DontSpeculateInt32;

return bytecodeCanTruncateInteger(add->arithNodeFlags()) ? SpeculateInt32AndTruncateConstants : DontSpeculateInt32;
if (bytecodeCanTruncateInteger(add->arithNodeFlags())) {
// If int32 + const double, then we should not speculate this add node with int32 type.
// Because ToInt32(int32 + const double) is not always equivalent to int32 + ToInt32(const double).
// For example:
// let the int32 = -1 and const double = 0.1, then ToInt32(-1 + 0.1) = 0 but -1 + ToInt32(0.1) = -1.
if (operandResultType == NodeResultInt32 && !isInteger(doubleImmediate))
return DontSpeculateInt32;
return SpeculateInt32AndTruncateConstants;
}

return DontSpeculateInt32;
}

B3::SparseCollection<Node> m_nodes;
Expand Down

0 comments on commit 60a14f2

Please sign in to comment.