Diff
Modified: trunk/JSTests/ChangeLog (244759 => 244760)
--- trunk/JSTests/ChangeLog 2019-04-29 22:06:05 UTC (rev 244759)
+++ trunk/JSTests/ChangeLog 2019-04-29 22:25:03 UTC (rev 244760)
@@ -1,3 +1,39 @@
+2019-04-29 Yusuke Suzuki <[email protected]>
+
+ normalizeMapKey should normalize NaN to one PureNaN bit pattern to make MapHash same
+ https://bugs.webkit.org/show_bug.cgi?id=197362
+
+ Reviewed by Saam Barati.
+
+ * stress/map-with-nan.js: Added.
+ (shouldBe):
+ (div):
+ (NaN1):
+ (NaN2):
+ (NaN3):
+ (NaN4):
+ (NaN1NoInline):
+ (NaN2NoInline):
+ (NaN3NoInline):
+ (NaN4NoInline):
+ (test1):
+ (test2):
+ (test3):
+ (test4):
+ * stress/set-with-nan.js: Added.
+ (shouldBe):
+ (div):
+ (NaN1):
+ (NaN2):
+ (NaN3):
+ (NaN4):
+ (NaN1NoInline):
+ (NaN2NoInline):
+ (NaN3NoInline):
+ (NaN4NoInline):
+ (test2):
+ (test4):
+
2019-04-26 Commit Queue <[email protected]>
Unreviewed, rolling out r244708.
Added: trunk/JSTests/stress/map-with-nan.js (0 => 244760)
--- trunk/JSTests/stress/map-with-nan.js (rev 0)
+++ trunk/JSTests/stress/map-with-nan.js 2019-04-29 22:25:03 UTC (rev 244760)
@@ -0,0 +1,91 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+function div(a, b) {
+ return a / b;
+}
+noInline(div);
+
+function NaN1() {
+ return div(0, 0);
+}
+function NaN2() {
+ return NaN;
+}
+function NaN3() {
+ return Infinity/Infinity;
+}
+function NaN4() {
+ return NaN + NaN;
+}
+
+function NaN1NoInline() {
+ return div(0, 0);
+}
+noInline(NaN1NoInline);
+function NaN2NoInline() {
+ return NaN;
+}
+noInline(NaN2NoInline);
+function NaN3NoInline() {
+ return Infinity/Infinity;
+}
+noInline(NaN3NoInline);
+function NaN4NoInline() {
+ return NaN + NaN;
+}
+noInline(NaN4NoInline);
+
+function test1()
+{
+ var map = new Map();
+ map.set(NaN1(), 1);
+ map.set(NaN2(), 2);
+ map.set(NaN3(), 3);
+ map.set(NaN4(), 4);
+ return map.size;
+}
+noInline(test1);
+
+function test2()
+{
+ return new Map([
+ [NaN1(), 1],
+ [NaN2(), 2],
+ [NaN3(), 3],
+ [NaN4(), 4]
+ ]).size;
+}
+noInline(test2);
+
+function test3()
+{
+ var map = new Map();
+ map.set(NaN1NoInline(), 1);
+ map.set(NaN2NoInline(), 2);
+ map.set(NaN3NoInline(), 3);
+ map.set(NaN4NoInline(), 4);
+ return map.size;
+}
+noInline(test3);
+
+function test4()
+{
+ return new Map([
+ [NaN1NoInline(), 1],
+ [NaN2NoInline(), 2],
+ [NaN3NoInline(), 3],
+ [NaN4NoInline(), 4]
+ ]).size;
+}
+noInline(test4);
+
+for (var i = 0; i < 1e5; ++i) {
+ shouldBe(test1(), 1);
+ shouldBe(test2(), 1);
+ shouldBe(test3(), 1);
+ shouldBe(test4(), 1);
+}
Added: trunk/JSTests/stress/set-with-nan.js (0 => 244760)
--- trunk/JSTests/stress/set-with-nan.js (rev 0)
+++ trunk/JSTests/stress/set-with-nan.js 2019-04-29 22:25:03 UTC (rev 244760)
@@ -0,0 +1,81 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+function div(a, b) {
+ return a / b;
+}
+noInline(div);
+
+function NaN1() {
+ return div(0, 0);
+}
+function NaN2() {
+ return NaN;
+}
+function NaN3() {
+ return Infinity/Infinity;
+}
+function NaN4() {
+ return NaN + NaN;
+}
+
+function NaN1NoInline() {
+ return div(0, 0);
+}
+noInline(NaN1NoInline);
+function NaN2NoInline() {
+ return NaN;
+}
+noInline(NaN2NoInline);
+function NaN3NoInline() {
+ return Infinity/Infinity;
+}
+noInline(NaN3NoInline);
+function NaN4NoInline() {
+ return NaN + NaN;
+}
+noInline(NaN4NoInline);
+
+function test1()
+{
+ var set = new Set();
+ set.add(NaN1());
+ set.add(NaN2());
+ set.add(NaN3());
+ set.add(NaN4());
+ return set.size;
+}
+noInline(test1);
+
+function test2()
+{
+ return new Set([NaN1(), NaN2(), NaN3(), NaN4()]).size;
+}
+noInline(test2);
+
+function test3()
+{
+ var set = new Set();
+ set.add(NaN1NoInline());
+ set.add(NaN2NoInline());
+ set.add(NaN3NoInline());
+ set.add(NaN4NoInline());
+ return set.size;
+}
+noInline(test3);
+
+function test4()
+{
+ return new Set([NaN1NoInline(), NaN2NoInline(), NaN3NoInline(), NaN4NoInline()]).size;
+}
+noInline(test4);
+
+for (var i = 0; i < 1e5; ++i) {
+ shouldBe(test1(), 1);
+ shouldBe(test2(), 1);
+ shouldBe(test3(), 1);
+ shouldBe(test4(), 1);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (244759 => 244760)
--- trunk/Source/_javascript_Core/ChangeLog 2019-04-29 22:06:05 UTC (rev 244759)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-04-29 22:25:03 UTC (rev 244760)
@@ -1,3 +1,22 @@
+2019-04-29 Yusuke Suzuki <[email protected]>
+
+ normalizeMapKey should normalize NaN to one PureNaN bit pattern to make MapHash same
+ https://bugs.webkit.org/show_bug.cgi?id=197362
+
+ Reviewed by Saam Barati.
+
+ Our Map/Set's hash algorithm relies on the bit pattern of JSValue. So our Map/Set has
+ normalization of the key, which normalizes Int32 / Double etc. But we did not normalize
+ pure NaNs into one canonicalized pure NaN. So we end up having multiple different pure NaNs
+ in one Map/Set. This patch normalizes NaN into one jsNaN(), which uses PNaN for the representation.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileNormalizeMapKey):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileNormalizeMapKey):
+ * runtime/HashMapImpl.h:
+ (JSC::normalizeMapKey):
+
2019-04-29 Alex Christensen <[email protected]>
<rdar://problem/50299396> Fix internal High Sierra build
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (244759 => 244760)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2019-04-29 22:06:05 UTC (rev 244759)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2019-04-29 22:25:03 UTC (rev 244760)
@@ -11714,6 +11714,7 @@
FPRReg tempFPR = temp.fpr();
CCallHelpers::JumpList passThroughCases;
+ CCallHelpers::JumpList doneCases;
passThroughCases.append(m_jit.branchIfNotNumber(keyRegs, scratchGPR));
passThroughCases.append(m_jit.branchIfInt32(keyRegs));
@@ -11723,19 +11724,22 @@
#else
unboxDouble(keyRegs.tagGPR(), keyRegs.payloadGPR(), doubleValueFPR, tempFPR);
#endif
- passThroughCases.append(m_jit.branchIfNaN(doubleValueFPR));
+ auto notNaN = m_jit.branchIfNotNaN(doubleValueFPR);
+ m_jit.moveTrustedValue(jsNaN(), resultRegs);
+ doneCases.append(m_jit.jump());
+ notNaN.link(&m_jit);
m_jit.truncateDoubleToInt32(doubleValueFPR, scratchGPR);
m_jit.convertInt32ToDouble(scratchGPR, tempFPR);
passThroughCases.append(m_jit.branchDouble(JITCompiler::DoubleNotEqual, doubleValueFPR, tempFPR));
m_jit.boxInt32(scratchGPR, resultRegs);
- auto done = m_jit.jump();
+ doneCases.append(m_jit.jump());
passThroughCases.link(&m_jit);
m_jit.moveValueRegs(keyRegs, resultRegs);
- done.link(&m_jit);
+ doneCases.link(&m_jit);
jsValueResult(resultRegs, node);
}
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (244759 => 244760)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2019-04-29 22:06:05 UTC (rev 244759)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2019-04-29 22:25:03 UTC (rev 244760)
@@ -9822,6 +9822,7 @@
m_out.appendTo(notInt32NumberCase, notNaNCase);
LValue doubleValue = unboxDouble(key);
+ ValueFromBlock normalizedNaNResult = m_out.anchor(m_out.constInt64(JSValue::encode(jsNaN())));
m_out.branch(m_out.doubleNotEqualOrUnordered(doubleValue, doubleValue), unsure(continuation), unsure(notNaNCase));
m_out.appendTo(notNaNCase, convertibleCase);
@@ -9830,11 +9831,11 @@
m_out.branch(m_out.doubleNotEqualOrUnordered(doubleValue, integerValueConvertedToDouble), unsure(continuation), unsure(convertibleCase));
m_out.appendTo(convertibleCase, continuation);
- ValueFromBlock slowResult = m_out.anchor(boxInt32(integerValue));
+ ValueFromBlock boxedIntResult = m_out.anchor(boxInt32(integerValue));
m_out.jump(continuation);
m_out.appendTo(continuation, lastNext);
- setJSValue(m_out.phi(Int64, fastResult, slowResult));
+ setJSValue(m_out.phi(Int64, fastResult, normalizedNaNResult, boxedIntResult));
}
void compileGetMapBucket()
Modified: trunk/Source/_javascript_Core/runtime/HashMapImpl.h (244759 => 244760)
--- trunk/Source/_javascript_Core/runtime/HashMapImpl.h 2019-04-29 22:06:05 UTC (rev 244759)
+++ trunk/Source/_javascript_Core/runtime/HashMapImpl.h 2019-04-29 22:25:03 UTC (rev 244760)
@@ -26,6 +26,7 @@
#pragma once
#include "ExceptionHelpers.h"
+#include "JSCJSValueInlines.h"
#include "JSObject.h"
namespace JSC {
@@ -245,7 +246,7 @@
double d = key.asDouble();
if (std::isnan(d))
- return key;
+ return jsNaN();
int i = static_cast<int>(d);
if (i == d) {
