Skip to content

Commit 0242919

Browse files
hyjorc1hyjorc1
committed
Negative exponent exception for BigInt not thrown in JIT compiler
https://bugs.webkit.org/show_bug.cgi?id=275939 rdar://131051084 Reviewed by Yusuke Suzuki. Some DFG nodes can have side effects even if their results are not used by others. In that case, we shouldn't clear the NodeMustGenerate flags unless they are proven not having side effects. * JSTests/stress/dead-code-eliminiation-with-side-effect-node.js: Added. (assert): (let.iteration.1e3.opt): (i.assert.opt.opt): (i.assert.opt): * Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): * Source/JavaScriptCore/dfg/DFGFixupPhase.cpp: (JSC::DFG::FixupPhase::fixupNode): * Source/JavaScriptCore/runtime/JSBigInt.h: * Source/JavaScriptCore/runtime/JSCJSValue.h: * Source/JavaScriptCore/runtime/JSCJSValueInlines.h: (JSC::JSValue::isZeroBigInt const): (JSC::JSValue::isNegativeBigInt const): Canonical link: https://commits.webkit.org/280841@main
1 parent 1bd46d2 commit 0242919

6 files changed

Lines changed: 184 additions & 10 deletions

File tree

‎JSTests/stress/dead-code-eliminiation-with-side-effect-node.js‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
//@ requireOptions("--jitPolicyScale=0.000005")
2+
3+
function assert(b) {
4+
if (!b)
5+
throw new Error("bad");
6+
}
7+
8+
let iteration = 1e3
9+
10+
/* -------------- Exponentiation -------------- */
11+
{
12+
function opt() {
13+
try {
14+
1n ** -1n;
15+
} catch (e) {
16+
return e
17+
}
18+
return 0;
19+
}
20+
21+
for (let i = 0; i < iteration; i++)
22+
assert(opt() != 0);
23+
}
24+
25+
{
26+
function opt(a, b) {
27+
try {
28+
a ** b;
29+
} catch (e) {
30+
return e
31+
}
32+
return 0;
33+
}
34+
35+
for (let i = 0; i < iteration; i++)
36+
assert(opt(1n, -1n) != 0);
37+
}
38+
39+
{
40+
function opt(b) {
41+
try {
42+
1n ** b;
43+
} catch (e) {
44+
return e
45+
}
46+
return 0;
47+
}
48+
49+
for (let i = 0; i < iteration; i++)
50+
assert(opt(-1n) != 0);
51+
}
52+
53+
/* -------------- Division -------------- */
54+
{
55+
function opt() {
56+
try {
57+
0n / 0n;
58+
} catch (e) {
59+
return e
60+
}
61+
return 0;
62+
}
63+
64+
for (let i = 0; i < iteration; i++)
65+
assert(opt() != 0);
66+
}
67+
68+
{
69+
function opt(a, b) {
70+
try {
71+
a / b;
72+
} catch (e) {
73+
return e
74+
}
75+
return 0;
76+
}
77+
78+
for (let i = 0; i < iteration; i++)
79+
assert(opt(0n, 0n) != 0);
80+
}
81+
82+
{
83+
function opt(b) {
84+
try {
85+
0n / b;
86+
} catch (e) {
87+
return e
88+
}
89+
return 0;
90+
}
91+
92+
for (let i = 0; i < iteration; i++)
93+
assert(opt(0n) != 0);
94+
}
95+
96+
/* -------------- Modulo -------------- */
97+
{
98+
function opt() {
99+
try {
100+
0n % 0n;
101+
} catch (e) {
102+
return e
103+
}
104+
return 0;
105+
}
106+
107+
for (let i = 0; i < iteration; i++)
108+
assert(opt() != 0);
109+
}
110+
111+
{
112+
function opt(a, b) {
113+
try {
114+
a % b;
115+
} catch (e) {
116+
return e
117+
}
118+
return 0;
119+
}
120+
121+
for (let i = 0; i < iteration; i++)
122+
assert(opt(0n, 0n) != 0);
123+
}
124+
125+
{
126+
function opt(b) {
127+
try {
128+
0n % b;
129+
} catch (e) {
130+
return e
131+
}
132+
return 0;
133+
}
134+
135+
for (let i = 0; i < iteration; i++)
136+
assert(opt(0n) != 0);
137+
}

‎Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,14 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
11001100
break;
11011101
}
11021102

1103+
bool isBigIntBinaryUsedKind = node->isBinaryUseKind(HeapBigIntUse) || node->isBinaryUseKind(AnyBigIntUse) || node->isBinaryUseKind(BigInt32Use);
1104+
if (node->mustGenerate() && isBigIntBinaryUsedKind) {
1105+
if (childY && childY.isBigInt() && !childY.isNegativeBigInt()) {
1106+
node->clearFlags(NodeMustGenerate);
1107+
m_state.setShouldTryConstantFolding(true);
1108+
}
1109+
}
1110+
11031111
if (node->isBinaryUseKind(HeapBigIntUse)) {
11041112
// FIXME: We will want an arithmetic mode here that allows us to speculate or dictate
11051113
// the format of our result:
@@ -1187,6 +1195,15 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
11871195
if (handleConstantDivOp(node))
11881196
break;
11891197

1198+
bool isBigIntBinaryUsedKind = node->isBinaryUseKind(HeapBigIntUse) || node->isBinaryUseKind(AnyBigIntUse) || node->isBinaryUseKind(BigInt32Use);
1199+
if (node->mustGenerate() && isBigIntBinaryUsedKind) {
1200+
JSValue left = forNode(node->child2()).value();
1201+
if (left && left.isBigInt() && !left.isZeroBigInt()) {
1202+
node->clearFlags(NodeMustGenerate);
1203+
m_state.setShouldTryConstantFolding(true);
1204+
}
1205+
}
1206+
11901207
if (node->isBinaryUseKind(HeapBigIntUse)) {
11911208
// FIXME: We will want an arithmetic mode here that allows us to speculate or dictate
11921209
// the format of our result:

‎Source/JavaScriptCore/dfg/DFGFixupPhase.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -739,14 +739,12 @@ class FixupPhase : public Phase {
739739
if (Node::shouldSpeculateHeapBigInt(leftChild.node(), rightChild.node())) {
740740
fixEdge<HeapBigIntUse>(leftChild);
741741
fixEdge<HeapBigIntUse>(rightChild);
742-
node->clearFlags(NodeMustGenerate);
743742
break;
744743
}
745744
#if USE(BIGINT32)
746745
if (Node::shouldSpeculateBigInt(leftChild.node(), rightChild.node())) {
747746
fixEdge<AnyBigIntUse>(leftChild);
748747
fixEdge<AnyBigIntUse>(rightChild);
749-
node->clearFlags(NodeMustGenerate);
750748
break;
751749
}
752750
#endif
@@ -818,14 +816,12 @@ class FixupPhase : public Phase {
818816
if (Node::shouldSpeculateHeapBigInt(node->child1().node(), node->child2().node())) {
819817
fixEdge<HeapBigIntUse>(node->child1());
820818
fixEdge<HeapBigIntUse>(node->child2());
821-
node->clearFlags(NodeMustGenerate);
822819
break;
823820
}
824821
#if USE(BIGINT32)
825822
if (Node::shouldSpeculateBigInt(node->child1().node(), node->child2().node())) {
826823
fixEdge<AnyBigIntUse>(node->child1());
827824
fixEdge<AnyBigIntUse>(node->child2());
828-
node->clearFlags(NodeMustGenerate);
829825
break;
830826
}
831827
#endif

‎Source/JavaScriptCore/runtime/JSBigInt.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,12 @@ class JSBigInt final : public JSCell {
493493

494494
static std::optional<double> tryExtractDouble(JSValue);
495495

496+
inline bool isZero() const
497+
{
498+
ASSERT(length() || !sign());
499+
return !length();
500+
}
501+
496502
private:
497503
JSBigInt(VM&, Structure*, Digit*, unsigned length);
498504

@@ -592,12 +598,6 @@ class JSBigInt final : public JSCell {
592598
static String toStringBasePowerOfTwo(VM&, JSGlobalObject*, JSBigInt*, unsigned radix);
593599
static String toStringGeneric(VM&, JSGlobalObject*, JSBigInt*, unsigned radix);
594600

595-
inline bool isZero() const
596-
{
597-
ASSERT(length() || !sign());
598-
return length() == 0;
599-
}
600-
601601
template <typename CharType>
602602
static JSValue parseInt(JSGlobalObject*, std::span<const CharType> data, ErrorParseMode);
603603

‎Source/JavaScriptCore/runtime/JSCJSValue.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,8 @@ class JSValue {
267267
bool isBigInt() const;
268268
bool isHeapBigInt() const;
269269
bool isBigInt32() const;
270+
bool isZeroBigInt() const;
271+
bool isNegativeBigInt() const;
270272
bool isSymbol() const;
271273
bool isPrimitive() const;
272274
bool isGetterSetter() const;

‎Source/JavaScriptCore/runtime/JSCJSValueInlines.h‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,28 @@ inline int32_t JSValue::bigInt32AsInt32() const
734734
}
735735
#endif // USE(BIGINT32)
736736

737+
inline bool JSValue::isZeroBigInt() const
738+
{
739+
ASSERT(isBigInt());
740+
#if USE(BIGINT32)
741+
if (isBigInt32())
742+
return !bigInt32AsInt32();
743+
#endif
744+
ASSERT(isHeapBigInt());
745+
return asHeapBigInt()->isZero();
746+
}
747+
748+
inline bool JSValue::isNegativeBigInt() const
749+
{
750+
ASSERT(isBigInt());
751+
#if USE(BIGINT32)
752+
if (isBigInt32())
753+
return bigInt32AsInt32() < 0;
754+
#endif
755+
ASSERT(isHeapBigInt());
756+
return asHeapBigInt()->sign();
757+
}
758+
737759
inline bool JSValue::isSymbol() const
738760
{
739761
return isCell() && asCell()->isSymbol();

0 commit comments

Comments
 (0)