Diff
Modified: trunk/JSTests/ChangeLog (225341 => 225342)
--- trunk/JSTests/ChangeLog 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/JSTests/ChangeLog 2017-11-30 20:48:53 UTC (rev 225342)
@@ -1,3 +1,32 @@
+2017-11-30 Yusuke Suzuki <[email protected]>
+
+ [DFG][FTL] operationHasIndexedProperty does not consider negative int32_t
+ https://bugs.webkit.org/show_bug.cgi?id=180190
+
+ Reviewed by Mark Lam.
+
+ * stress/operation-in-may-have-negative-int32-array-storage.js: Added.
+ (shouldBe):
+ (test1):
+ * stress/operation-in-may-have-negative-int32-contiguous-array.js: Added.
+ (shouldBe):
+ (test1):
+ * stress/operation-in-may-have-negative-int32-double-array.js: Added.
+ (shouldBe):
+ (test1):
+ * stress/operation-in-may-have-negative-int32-generic-array.js: Added.
+ (shouldBe):
+ (test1):
+ * stress/operation-in-may-have-negative-int32-int32-array.js: Added.
+ (shouldBe):
+ (test1):
+ * stress/operation-in-may-have-negative-int32.js: Added.
+ (shouldBe):
+ (test2):
+ * stress/operation-in-negative-int32-cast.js: Added.
+ (shouldBe):
+ (test1):
+
2017-11-28 JF Bastien <[email protected]>
Strict and sloppy functions shouldn't share structure
Added: trunk/JSTests/stress/operation-in-may-have-negative-int32-array-storage.js (0 => 225342)
--- trunk/JSTests/stress/operation-in-may-have-negative-int32-array-storage.js (rev 0)
+++ trunk/JSTests/stress/operation-in-may-have-negative-int32-array-storage.js 2017-11-30 20:48:53 UTC (rev 225342)
@@ -0,0 +1,19 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var k = -1;
+var o1 = [20];
+o1[k] = 42;
+ensureArrayStorage(o1);
+
+function test1(o)
+{
+ return k in o;
+}
+noInline(test1);
+
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test1(o1), true);
Added: trunk/JSTests/stress/operation-in-may-have-negative-int32-contiguous-array.js (0 => 225342)
--- trunk/JSTests/stress/operation-in-may-have-negative-int32-contiguous-array.js (rev 0)
+++ trunk/JSTests/stress/operation-in-may-have-negative-int32-contiguous-array.js 2017-11-30 20:48:53 UTC (rev 225342)
@@ -0,0 +1,18 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var k = -1;
+var o1 = ["Cocoa"];
+o1[k] = 42;
+
+function test1(o)
+{
+ return k in o;
+}
+noInline(test1);
+
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test1(o1), true);
Added: trunk/JSTests/stress/operation-in-may-have-negative-int32-double-array.js (0 => 225342)
--- trunk/JSTests/stress/operation-in-may-have-negative-int32-double-array.js (rev 0)
+++ trunk/JSTests/stress/operation-in-may-have-negative-int32-double-array.js 2017-11-30 20:48:53 UTC (rev 225342)
@@ -0,0 +1,18 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var k = -1;
+var o1 = [42.5];
+o1[k] = 300.2;
+
+function test1(o)
+{
+ return k in o;
+}
+noInline(test1);
+
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test1(o1), true);
Added: trunk/JSTests/stress/operation-in-may-have-negative-int32-generic-array.js (0 => 225342)
--- trunk/JSTests/stress/operation-in-may-have-negative-int32-generic-array.js (rev 0)
+++ trunk/JSTests/stress/operation-in-may-have-negative-int32-generic-array.js 2017-11-30 20:48:53 UTC (rev 225342)
@@ -0,0 +1,18 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var k = -1;
+var o1 = [];
+o1[k] = 42;
+
+function test1(o)
+{
+ return k in o;
+}
+noInline(test1);
+
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test1(o1), true);
Added: trunk/JSTests/stress/operation-in-may-have-negative-int32-int32-array.js (0 => 225342)
--- trunk/JSTests/stress/operation-in-may-have-negative-int32-int32-array.js (rev 0)
+++ trunk/JSTests/stress/operation-in-may-have-negative-int32-int32-array.js 2017-11-30 20:48:53 UTC (rev 225342)
@@ -0,0 +1,18 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var k = -1;
+var o1 = [20];
+o1[k] = 42;
+
+function test1(o)
+{
+ return k in o;
+}
+noInline(test1);
+
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test1(o1), true);
Added: trunk/JSTests/stress/operation-in-may-have-negative-int32.js (0 => 225342)
--- trunk/JSTests/stress/operation-in-may-have-negative-int32.js (rev 0)
+++ trunk/JSTests/stress/operation-in-may-have-negative-int32.js 2017-11-30 20:48:53 UTC (rev 225342)
@@ -0,0 +1,32 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var k = -1;
+var o1 = {};
+o1[k] = true;
+var o2 = {};
+
+function test1(o)
+{
+ return k in o;
+}
+noInline(test1);
+
+function test2(o)
+{
+ return k in o;
+}
+noInline(test2);
+
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test1(o1), true);
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test1(o2), false);
+
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test2(o2), false);
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test2(o1), true);
Added: trunk/JSTests/stress/operation-in-negative-int32-cast.js (0 => 225342)
--- trunk/JSTests/stress/operation-in-negative-int32-cast.js (rev 0)
+++ trunk/JSTests/stress/operation-in-negative-int32-cast.js 2017-11-30 20:48:53 UTC (rev 225342)
@@ -0,0 +1,20 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var INT32_MIN = -2147483648;
+var INT32_MIN_IN_UINT32 = 0x80000000;
+var o1 = [];
+o1[INT32_MIN_IN_UINT32] = true;
+ensureArrayStorage(o1);
+
+function test1(o, key)
+{
+ return key in o;
+}
+noInline(test1);
+
+for (var i = 0; i < 1e6; ++i)
+ shouldBe(test1(o1, INT32_MIN), false);
Modified: trunk/Source/_javascript_Core/ChangeLog (225341 => 225342)
--- trunk/Source/_javascript_Core/ChangeLog 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-11-30 20:48:53 UTC (rev 225342)
@@ -1,3 +1,37 @@
+2017-11-30 Yusuke Suzuki <[email protected]>
+
+ [DFG][FTL] operationHasIndexedProperty does not consider negative int32_t
+ https://bugs.webkit.org/show_bug.cgi?id=180190
+
+ Reviewed by Mark Lam.
+
+ If DFG HasIndexedProperty node observes negative index, it goes to a slow
+ path by calling operationHasIndexedProperty. The problem is that
+ operationHasIndexedProperty does not account negative index. Negative index
+ was used as uint32 array index.
+
+ In this patch we add a path for negative index in operationHasIndexedProperty.
+ And rename it to operationHasIndexedPropertyByInt to make intension clear.
+ We also move operationHasIndexedPropertyByInt from JITOperations to DFGOperations
+ since it is only used in DFG and FTL.
+
+ While fixing this bug, we found that our op_in does not record OutOfBound feedback.
+ This causes repeated OSR exit and significantly regresses the performance. We opened
+ a bug to track this issue[1].
+
+ [1]: https://bugs.webkit.org/show_bug.cgi?id=180192
+
+ * dfg/DFGOperations.cpp:
+ * dfg/DFGOperations.h:
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileHasIndexedProperty):
+ * jit/JITOperations.cpp:
+ * jit/JITOperations.h:
+
2017-11-30 Michael Saboff <[email protected]>
Allow JSC command line tool to accept UTF8
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (225341 => 225342)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2017-11-30 20:48:53 UTC (rev 225342)
@@ -634,7 +634,7 @@
NativeCallFrameTracer tracer(vm, exec);
if (index < 0) {
- // Go the slowest way possible becase negative indices don't use indexed storage.
+ // Go the slowest way possible because negative indices don't use indexed storage.
return JSValue::encode(JSValue(base).get(exec, Identifier::from(exec, index)));
}
@@ -1808,6 +1808,18 @@
return reinterpret_cast<char*>(asObject(cell)->ensureArrayStorage(vm));
}
+EncodedJSValue JIT_OPERATION operationHasIndexedPropertyByInt(ExecState* exec, JSCell* baseCell, int32_t subscript, int32_t internalMethodType)
+{
+ VM& vm = exec->vm();
+ NativeCallFrameTracer tracer(&vm, exec);
+ JSObject* object = baseCell->toObject(exec, exec->lexicalGlobalObject());
+ if (UNLIKELY(subscript < 0)) {
+ // Go the slowest way possible because negative indices don't use indexed storage.
+ return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, Identifier::from(exec, subscript), static_cast<PropertySlot::InternalMethodType>(internalMethodType))));
+ }
+ return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, subscript, static_cast<PropertySlot::InternalMethodType>(internalMethodType))));
+}
+
StringImpl* JIT_OPERATION operationResolveRope(ExecState* exec, JSString* string)
{
VM& vm = exec->vm();
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.h (225341 => 225342)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.h 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.h 2017-11-30 20:48:53 UTC (rev 225342)
@@ -80,6 +80,7 @@
EncodedJSValue JIT_OPERATION operationGetByValWithThis(ExecState*, EncodedJSValue, EncodedJSValue, EncodedJSValue) WTF_INTERNAL;
EncodedJSValue JIT_OPERATION operationGetPrototypeOf(ExecState*, EncodedJSValue) WTF_INTERNAL;
EncodedJSValue JIT_OPERATION operationGetPrototypeOfObject(ExecState*, JSObject*) WTF_INTERNAL;
+EncodedJSValue JIT_OPERATION operationHasIndexedPropertyByInt(ExecState*, JSCell*, int32_t, int32_t);
char* JIT_OPERATION operationNewArray(ExecState*, Structure*, void*, size_t) WTF_INTERNAL;
char* JIT_OPERATION operationNewArrayBuffer(ExecState*, Structure*, size_t, size_t) WTF_INTERNAL;
char* JIT_OPERATION operationNewEmptyArray(ExecState*, Structure*) WTF_INTERNAL;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (225341 => 225342)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2017-11-30 20:48:53 UTC (rev 225342)
@@ -5271,7 +5271,7 @@
moveTrueTo(resultPayloadGPR);
MacroAssembler::Jump done = m_jit.jump();
- addSlowPathGenerator(slowPathCall(slowCases, this, operationHasIndexedProperty, JSValueRegs(resultTagGPR, resultPayloadGPR), baseGPR, indexGPR, static_cast<int32_t>(node->internalMethodType())));
+ addSlowPathGenerator(slowPathCall(slowCases, this, operationHasIndexedPropertyByInt, JSValueRegs(resultTagGPR, resultPayloadGPR), baseGPR, indexGPR, static_cast<int32_t>(node->internalMethodType())));
done.link(&m_jit);
booleanResult(resultPayloadGPR, node);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (225341 => 225342)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2017-11-30 20:48:53 UTC (rev 225342)
@@ -5733,7 +5733,7 @@
}
}
- addSlowPathGenerator(slowPathCall(slowCases, this, operationHasIndexedProperty, resultGPR, baseGPR, indexGPR, static_cast<int32_t>(node->internalMethodType())));
+ addSlowPathGenerator(slowPathCall(slowCases, this, operationHasIndexedPropertyByInt, resultGPR, baseGPR, indexGPR, static_cast<int32_t>(node->internalMethodType())));
jsValueResult(resultGPR, node, DataFormatJSBoolean);
break;
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (225341 => 225342)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-11-30 20:48:53 UTC (rev 225342)
@@ -9428,7 +9428,7 @@
m_out.appendTo(slowCase, continuation);
ValueFromBlock slowResult = m_out.anchor(m_out.equal(
m_out.constInt64(JSValue::encode(jsBoolean(true))),
- vmCall(Int64, m_out.operation(operationHasIndexedProperty), m_callFrame, base, index, internalMethodType)));
+ vmCall(Int64, m_out.operation(operationHasIndexedPropertyByInt), m_callFrame, base, index, internalMethodType)));
m_out.jump(continuation);
m_out.appendTo(continuation, lastNext);
@@ -9464,7 +9464,7 @@
m_out.appendTo(slowCase, continuation);
ValueFromBlock slowResult = m_out.anchor(m_out.equal(
m_out.constInt64(JSValue::encode(jsBoolean(true))),
- vmCall(Int64, m_out.operation(operationHasIndexedProperty), m_callFrame, base, index, internalMethodType)));
+ vmCall(Int64, m_out.operation(operationHasIndexedPropertyByInt), m_callFrame, base, index, internalMethodType)));
m_out.jump(continuation);
m_out.appendTo(continuation, lastNext);
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (225341 => 225342)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2017-11-30 20:48:53 UTC (rev 225342)
@@ -2398,14 +2398,6 @@
return JSValue::encode(jsBoolean(base->hasPropertyGeneric(exec, asString(propertyName)->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty)));
}
-EncodedJSValue JIT_OPERATION operationHasIndexedProperty(ExecState* exec, JSCell* baseCell, int32_t subscript, int32_t internalMethodType)
-{
- VM& vm = exec->vm();
- NativeCallFrameTracer tracer(&vm, exec);
- JSObject* object = baseCell->toObject(exec, exec->lexicalGlobalObject());
- return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, subscript, static_cast<PropertySlot::InternalMethodType>(internalMethodType))));
-}
-
JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState* exec, JSCell* cell)
{
VM& vm = exec->vm();
Modified: trunk/Source/_javascript_Core/jit/JITOperations.h (225341 => 225342)
--- trunk/Source/_javascript_Core/jit/JITOperations.h 2017-11-30 20:40:04 UTC (rev 225341)
+++ trunk/Source/_javascript_Core/jit/JITOperations.h 2017-11-30 20:48:53 UTC (rev 225342)
@@ -461,7 +461,6 @@
int32_t JIT_OPERATION operationInstanceOfCustom(ExecState*, EncodedJSValue encodedValue, JSObject* constructor, EncodedJSValue encodedHasInstance) WTF_INTERNAL;
EncodedJSValue JIT_OPERATION operationHasGenericProperty(ExecState*, EncodedJSValue, JSCell*);
-EncodedJSValue JIT_OPERATION operationHasIndexedProperty(ExecState*, JSCell*, int32_t, int32_t);
JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState*, JSCell*);
EncodedJSValue JIT_OPERATION operationNextEnumeratorPname(ExecState*, JSCell*, int32_t);
JSCell* JIT_OPERATION operationToIndexString(ExecState*, int32_t);
