Diff
Modified: trunk/JSTests/ChangeLog (272404 => 272405)
--- trunk/JSTests/ChangeLog 2021-02-05 07:40:33 UTC (rev 272404)
+++ trunk/JSTests/ChangeLog 2021-02-05 08:45:09 UTC (rev 272405)
@@ -1,3 +1,26 @@
+2021-02-05 Yusuke Suzuki <[email protected]>
+
+ [JSC] Atomics.store in DFG / FTL should return ToNumber(input) value
+ https://bugs.webkit.org/show_bug.cgi?id=221438
+ <rdar://problem/73973264>
+
+ Reviewed by Filip Pizlo.
+
+ * stress/atomic-store-result-type-in-ai.js: Added.
+ * stress/atomics-store-result-double-nan.js: Added.
+ (shouldBe):
+ (test):
+ * stress/atomics-store-result-double-negative-zero.js: Added.
+ (shouldBe):
+ (test):
+ * stress/atomics-store-result-double.js: Added.
+ (shouldBe):
+ (test):
+ * stress/atomics-store-result-int52.js: Added.
+ (shouldBe):
+ * stress/atomics-store-result.js: Added.
+ (shouldBe):
+
2021-02-03 Yusuke Suzuki <[email protected]>
[JSC] Update test262
Added: trunk/JSTests/stress/atomic-store-result-type-in-ai.js (0 => 272405)
--- trunk/JSTests/stress/atomic-store-result-type-in-ai.js (rev 0)
+++ trunk/JSTests/stress/atomic-store-result-type-in-ai.js 2021-02-05 08:45:09 UTC (rev 272405)
@@ -0,0 +1,5 @@
+//@ runDefault("--forceDebuggerBytecodeGeneration=1", "--useConcurrentJIT=0")
+let a = new Uint8Array(1);
+for (let i = 0; i < 10000; ++i) {
+ Atomics.store(a, 0, i);
+}
Added: trunk/JSTests/stress/atomics-store-result-double-nan.js (0 => 272405)
--- trunk/JSTests/stress/atomics-store-result-double-nan.js (rev 0)
+++ trunk/JSTests/stress/atomics-store-result-double-nan.js 2021-02-05 08:45:09 UTC (rev 272405)
@@ -0,0 +1,13 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function test(i8, double) {
+ shouldBe(Atomics.store(i8, 0, double), 0);
+}
+noInline(test);
+
+var i8 = new Int8Array(new SharedArrayBuffer(8));
+for (var i = 0; i < 1e4; ++i)
+ test(i8, NaN);
Added: trunk/JSTests/stress/atomics-store-result-double-negative-zero.js (0 => 272405)
--- trunk/JSTests/stress/atomics-store-result-double-negative-zero.js (rev 0)
+++ trunk/JSTests/stress/atomics-store-result-double-negative-zero.js 2021-02-05 08:45:09 UTC (rev 272405)
@@ -0,0 +1,13 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function test(i8, double) {
+ shouldBe(Math.sign(Atomics.store(i8, 0, double)), 0);
+}
+noInline(test);
+
+var i8 = new Int8Array(new SharedArrayBuffer(8));
+for (var i = 0; i < 1e4; ++i)
+ test(i8, -0.0);
Added: trunk/JSTests/stress/atomics-store-result-double.js (0 => 272405)
--- trunk/JSTests/stress/atomics-store-result-double.js (rev 0)
+++ trunk/JSTests/stress/atomics-store-result-double.js 2021-02-05 08:45:09 UTC (rev 272405)
@@ -0,0 +1,15 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+function test(i8, double) {
+ shouldBe(Atomics.store(i8, 0, double), double);
+}
+noInline(test);
+
+var i8 = new Int8Array(32);
+for (var i = 0; i < 1e4; ++i) {
+ test(i8, Infinity);
+ test(i8, -Infinity);
+}
Added: trunk/JSTests/stress/atomics-store-result-int52.js (0 => 272405)
--- trunk/JSTests/stress/atomics-store-result-int52.js (rev 0)
+++ trunk/JSTests/stress/atomics-store-result-int52.js 2021-02-05 08:45:09 UTC (rev 272405)
@@ -0,0 +1,8 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var i8 = new Int8Array(32);
+for (var i = 0; i < 1e4; ++i)
+ shouldBe(Atomics.store(i8, 0, 0xffffffff00 + i), 0xffffffff00 + i);
Added: trunk/JSTests/stress/atomics-store-result.js (0 => 272405)
--- trunk/JSTests/stress/atomics-store-result.js (rev 0)
+++ trunk/JSTests/stress/atomics-store-result.js 2021-02-05 08:45:09 UTC (rev 272405)
@@ -0,0 +1,8 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var i8 = new Int8Array(32);
+for (var i = 0; i < 1e4; ++i)
+ shouldBe(Atomics.store(i8, 0, 0xff00 + i), 0xff00 + i);
Modified: trunk/Source/_javascript_Core/ChangeLog (272404 => 272405)
--- trunk/Source/_javascript_Core/ChangeLog 2021-02-05 07:40:33 UTC (rev 272404)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-02-05 08:45:09 UTC (rev 272405)
@@ -1,3 +1,36 @@
+2021-02-05 Yusuke Suzuki <[email protected]>
+
+ [JSC] Atomics.store in DFG / FTL should return ToNumber(input) value
+ https://bugs.webkit.org/show_bug.cgi?id=221438
+ <rdar://problem/73973264>
+
+ Reviewed by Filip Pizlo.
+
+ Atomics.store is different from the other ReadModifyWrite atomics. It returns the input value without truncating it into TypedArray's requirement.
+ For example,
+
+ var u8 = new Uint8Array(8);
+ Atomics.store(u8, 0, 0xffff) === 0xffff // Not 0xff.
+
+ However DFG and FTL implementations do not handle it correctly.
+ This patch fixes AI, fixup, and code generations to handle this.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):
+ (JSC::DFG::SpeculativeJIT::getIntTypedArrayStoreOperandForAtomics):
+ * dfg/DFGSpeculativeJIT.h:
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileAtomicsReadModifyWrite):
+ (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
+ (JSC::FTL::DFG::LowerDFGToB3::setIntTypedArrayLoadResult):
+ (JSC::FTL::DFG::LowerDFGToB3::toIntegerOrInfinity):
+
2021-02-04 Yusuke Suzuki <[email protected]>
[JSC] Implement Object.entries in C++
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (272404 => 272405)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2021-02-05 07:40:33 UTC (rev 272404)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2021-02-05 08:45:09 UTC (rev 272405)
@@ -2324,6 +2324,44 @@
if (!storageEdge)
clobberWorld();
}
+
+ if (node->op() == AtomicsStore) {
+ // The returned value from Atomics.store does not rely on typed array types. It is relying
+ // on input's UseKind. For example,
+ //
+ // Atomics.store(uint8Array, /* index */ 0, Infinity) // => returned value is Infinity
+ //
+ // Since the other ReadModifyWrite atomics return values stored in the typed array previously,
+ // the returned values rely on the typed array types. On the other hand, Atomics.store's
+ // returned value is input value. This means that Atomics.store + Uint8Array can return doubles
+ // while the typed array is Uint8Array (the above one is the example).
+ switch (node->arrayMode().type()) {
+ case Array::Generic:
+ clobberWorld();
+ makeHeapTopForNode(node);
+ break;
+ default: {
+ Edge operand = m_graph.child(node, 2);
+ switch (operand.useKind()) {
+ case Int32Use:
+ setNonCellTypeForNode(node, SpecInt32Only);
+ break;
+ case Int52RepUse:
+ setNonCellTypeForNode(node, SpecInt52Any);
+ break;
+ case DoubleRepUse:
+ setNonCellTypeForNode(node, SpecFullDouble);
+ break;
+ default:
+ DFG_CRASH(m_graph, node, "Bad use kind");
+ break;
+ }
+ break;
+ }
+ }
+ break;
+ }
+
switch (node->arrayMode().type()) {
case Array::SelectUsingPredictions:
case Array::Unprofiled:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (272404 => 272405)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2021-02-05 07:40:33 UTC (rev 272404)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2021-02-05 08:45:09 UTC (rev 272405)
@@ -1379,10 +1379,26 @@
}
blessArrayOperation(base, index, m_graph.child(node, 2 + numExtraAtomicsArgs(node->op())));
- if (node->arrayMode().type() != Array::Generic) {
- fixEdge<CellUse>(base);
- fixEdge<Int32Use>(index);
+ fixEdge<CellUse>(base);
+ fixEdge<Int32Use>(index);
+ if (node->op() == AtomicsStore) {
+ Edge& operand = m_graph.child(node, 2);
+ switch (operand.useKind()) {
+ case Int32Use:
+ // Default result type.
+ break;
+ case Int52RepUse:
+ node->setResult(NodeResultInt52);
+ break;
+ case DoubleRepUse:
+ node->setResult(NodeResultDouble);
+ break;
+ default:
+ DFG_CRASH(m_graph, node, "Bad use kind");
+ break;
+ }
+ } else {
if (node->arrayMode().type() == Array::Uint32Array) {
// NOTE: This means basically always doing Int52.
if (node->shouldSpeculateInt52())
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (272404 => 272405)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2021-02-05 07:40:33 UTC (rev 272404)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2021-02-05 08:45:09 UTC (rev 272405)
@@ -3145,7 +3145,7 @@
emitTypedArrayBoundsCheck(node, baseReg, propertyReg);
loadFromIntTypedArray(storageReg, propertyReg, resultReg, type);
- bool canSpeculate = true;
+ constexpr bool canSpeculate = true;
setIntTypedArrayLoadResult(node, resultReg, type, canSpeculate);
}
@@ -3271,6 +3271,31 @@
return true;
}
+bool SpeculativeJIT::getIntTypedArrayStoreOperandForAtomics(
+ GPRTemporary& value,
+ GPRReg property,
+#if USE(JSVALUE32_64)
+ GPRTemporary& propertyTag,
+ GPRTemporary& valueTag,
+#endif
+ Edge valueUse)
+{
+ JITCompiler::JumpList slowPathCases;
+ constexpr bool isClamped = false;
+ bool result = getIntTypedArrayStoreOperand(
+ value,
+ property,
+#if USE(JSVALUE32_64)
+ propertyTag,
+ valueTag,
+#endif
+ valueUse,
+ slowPathCases,
+ isClamped);
+ ASSERT(slowPathCases.empty());
+ return result;
+}
+
void SpeculativeJIT::compilePutByValForIntTypedArray(GPRReg base, GPRReg property, Node* node, TypedArrayType type)
{
ASSERT(isInt(type));
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (272404 => 272405)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2021-02-05 07:40:33 UTC (rev 272404)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2021-02-05 08:45:09 UTC (rev 272405)
@@ -1385,8 +1385,16 @@
GPRTemporary& valueTag,
#endif
Edge valueUse, JITCompiler::JumpList& slowPathCases, bool isClamped = false);
+ bool getIntTypedArrayStoreOperandForAtomics(
+ GPRTemporary& value,
+ GPRReg property,
+#if USE(JSVALUE32_64)
+ GPRTemporary& propertyTag,
+ GPRTemporary& valueTag,
+#endif
+ Edge valueUse);
void loadFromIntTypedArray(GPRReg storageReg, GPRReg propertyReg, GPRReg resultReg, TypedArrayType);
- void setIntTypedArrayLoadResult(Node*, GPRReg resultReg, TypedArrayType, bool canSpeculate = false);
+ void setIntTypedArrayLoadResult(Node*, GPRReg resultReg, TypedArrayType, bool canSpeculate);
template <typename ClassType> void compileNewFunctionCommon(GPRReg, RegisteredStructure, GPRReg, GPRReg, GPRReg, MacroAssembler::JumpList&, size_t, FunctionExecutable*);
void compileNewFunction(Node*);
void compileSetFunctionName(Node*);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (272404 => 272405)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2021-02-05 07:40:33 UTC (rev 272404)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2021-02-05 08:45:09 UTC (rev 272405)
@@ -3243,43 +3243,43 @@
GPRReg argGPRs[2];
GPRReg resultGPR;
- auto callSlowPath = [&] () {
- auto globalObjectImmPtr = TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic));
- switch (node->op()) {
- case AtomicsAdd:
- callOperation(operationAtomicsAdd, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
- break;
- case AtomicsAnd:
- callOperation(operationAtomicsAnd, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
- break;
- case AtomicsCompareExchange:
- callOperation(operationAtomicsCompareExchange, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0], argGPRs[1]);
- break;
- case AtomicsExchange:
- callOperation(operationAtomicsExchange, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
- break;
- case AtomicsLoad:
- callOperation(operationAtomicsLoad, resultGPR, globalObjectImmPtr, baseGPR, indexGPR);
- break;
- case AtomicsOr:
- callOperation(operationAtomicsOr, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
- break;
- case AtomicsStore:
- callOperation(operationAtomicsStore, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
- break;
- case AtomicsSub:
- callOperation(operationAtomicsSub, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
- break;
- case AtomicsXor:
- callOperation(operationAtomicsXor, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
- break;
- default:
- RELEASE_ASSERT_NOT_REACHED();
- break;
- }
- };
-
if (!storageEdge) {
+ auto callSlowPath = [&] () {
+ auto globalObjectImmPtr = TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic));
+ switch (node->op()) {
+ case AtomicsAdd:
+ callOperation(operationAtomicsAdd, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
+ break;
+ case AtomicsAnd:
+ callOperation(operationAtomicsAnd, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
+ break;
+ case AtomicsCompareExchange:
+ callOperation(operationAtomicsCompareExchange, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0], argGPRs[1]);
+ break;
+ case AtomicsExchange:
+ callOperation(operationAtomicsExchange, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
+ break;
+ case AtomicsLoad:
+ callOperation(operationAtomicsLoad, resultGPR, globalObjectImmPtr, baseGPR, indexGPR);
+ break;
+ case AtomicsOr:
+ callOperation(operationAtomicsOr, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
+ break;
+ case AtomicsStore:
+ callOperation(operationAtomicsStore, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
+ break;
+ case AtomicsSub:
+ callOperation(operationAtomicsSub, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
+ break;
+ case AtomicsXor:
+ callOperation(operationAtomicsXor, resultGPR, globalObjectImmPtr, baseGPR, indexGPR, argGPRs[0]);
+ break;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ break;
+ }
+ };
+
// We are in generic mode!
JSValueOperand base(this, baseEdge);
JSValueOperand index(this, indexEdge);
@@ -3313,11 +3313,9 @@
GPRTemporary args[2];
- JITCompiler::JumpList slowPathCases;
-
bool ok = true;
for (unsigned i = numExtraArgs; i--;) {
- if (!getIntTypedArrayStoreOperand(args[i], indexGPR, argEdges[i], slowPathCases)) {
+ if (!getIntTypedArrayStoreOperandForAtomics(args[i], indexGPR, argEdges[i])) {
noResult(node);
ok = false;
}
@@ -3417,21 +3415,26 @@
}
m_jit.jump().linkTo(loop, &m_jit);
- if (!slowPathCases.empty()) {
- slowPathCases.link(&m_jit);
- silentSpillAllRegisters(resultGPR);
- // Since we spilled, we can do things to registers.
- m_jit.boxCell(baseGPR, JSValueRegs(baseGPR));
- m_jit.boxInt32(indexGPR, JSValueRegs(indexGPR));
- for (unsigned i = numExtraArgs; i--;)
- m_jit.boxInt32(argGPRs[i], JSValueRegs(argGPRs[i]));
- callSlowPath();
- silentFillAllRegisters();
- m_jit.exceptionCheck();
+ success.link(&m_jit);
+
+ if (node->op() == AtomicsStore) {
+ Edge operand = argEdges[0];
+ switch (operand.useKind()) {
+ case Int32Use:
+ m_jit.zeroExtend32ToWord(resultGPR, resultGPR);
+ strictInt32Result(resultGPR, node);
+ break;
+ case Int52RepUse:
+ strictInt52Result(resultGPR, node);
+ break;
+ default:
+ DFG_CRASH(m_graph, node, "Bad result type");
+ break;
+ }
+ break;
}
-
- success.link(&m_jit);
- setIntTypedArrayLoadResult(node, resultGPR, type);
+ constexpr bool canSpeculate = false;
+ setIntTypedArrayLoadResult(node, resultGPR, type, canSpeculate);
break;
}
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (272404 => 272405)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2021-02-05 07:40:33 UTC (rev 272404)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2021-02-05 08:45:09 UTC (rev 272405)
@@ -4275,7 +4275,27 @@
// We have to keep base alive since that keeps storage alive.
ensureStillAliveHere(lowCell(baseEdge));
- setIntTypedArrayLoadResult(result, type);
+
+ if (m_node->op() == AtomicsStore) {
+ Edge operand = argEdges[0];
+ switch (operand.useKind()) {
+ case Int32Use:
+ setInt32(lowInt32(operand));
+ break;
+ case Int52RepUse:
+ setStrictInt52(lowStrictInt52(operand));
+ break;
+ case DoubleRepUse:
+ setDouble(toIntegerOrInfinity(lowDouble(operand)));
+ break;
+ default:
+ DFG_CRASH(m_graph, m_node, "Bad result type");
+ break;
+ }
+ return;
+ }
+ constexpr bool canSpeculate = false;
+ setIntTypedArrayLoadResult(result, type, canSpeculate);
}
void compileAtomicsIsLockFree()
@@ -5247,7 +5267,7 @@
LValue result = loadFromIntTypedArray(pointer, type);
// We have to keep base alive since that keeps storage alive.
ensureStillAliveHere(base);
- bool canSpeculate = true;
+ constexpr bool canSpeculate = true;
setIntTypedArrayLoadResult(result, type, canSpeculate);
return;
}
@@ -16892,7 +16912,7 @@
}
}
- void setIntTypedArrayLoadResult(LValue result, TypedArrayType type, bool canSpeculate = false)
+ void setIntTypedArrayLoadResult(LValue result, TypedArrayType type, bool canSpeculate)
{
if (elementSize(type) < 4 || isSigned(type)) {
setInt32(result);
@@ -19634,6 +19654,14 @@
return m_out.addPtr(immutableButterfly, JSImmutableButterfly::offsetOfData());
}
+ LValue toIntegerOrInfinity(LValue doubleValue)
+ {
+ // https://tc39.es/ecma262/#sec-tointegerorinfinity
+ // 1. If value is either of +0, -0, or NaN, return +0
+ // 2. Otherwise, return trunc(value)
+ return m_out.select(m_out.doubleNotEqualAndOrdered(doubleValue, m_out.doubleZero), m_out.doubleTrunc(doubleValue), m_out.doubleZero);
+ }
+
void addWeakReference(JSCell* target)
{
m_graph.m_plan.weakReferences().addLazily(target);
