Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (201455 => 201456)
--- trunk/Source/_javascript_Core/ChangeLog 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-05-27 18:36:30 UTC (rev 201456)
@@ -1,3 +1,48 @@
+2016-05-27 Keith Miller <[email protected]>
+
+ get_by_id should support caching unset properties in the LLInt
+ https://bugs.webkit.org/show_bug.cgi?id=158136
+
+ Reviewed by Benjamin Poulain.
+
+ Recently, we started supporting prototype load caching for get_by_id
+ in the LLInt. This patch extends that to caching unset properties.
+ While it is uncommon in general for a program to see a single structure
+ without a given property, the Array.prototype.concat function needs to
+ lookup the Symbol.isConcatSpreadable property. For any existing code
+ That property will never be set as it did not exist prior to ES6.
+
+ Similarly to the get_by_id_proto_load bytecode, this patch adds a new
+ bytecode, get_by_id_unset that checks the structureID of the base and
+ assigns undefined to the result.
+
+ There are no new tests here since we already have many tests that
+ incidentally cover this change.
+
+ * bytecode/BytecodeList.json:
+ * bytecode/BytecodeUseDef.h:
+ (JSC::computeUsesForBytecodeOffset):
+ (JSC::computeDefsForBytecodeOffset):
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::printGetByIdOp):
+ (JSC::CodeBlock::dumpBytecode):
+ (JSC::CodeBlock::finalizeLLIntInlineCaches):
+ * bytecode/GetByIdStatus.cpp:
+ (JSC::GetByIdStatus::computeFromLLInt):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGCapabilities.cpp:
+ (JSC::DFG::capabilityLevel):
+ * jit/JIT.cpp:
+ (JSC::JIT::privateCompileMainPass):
+ (JSC::JIT::privateCompileSlowCases):
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::setupGetByIdPrototypeCache):
+ (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+ * llint/LLIntSlowPaths.h:
+ * llint/LowLevelInterpreter32_64.asm:
+ * llint/LowLevelInterpreter64.asm:
+
2016-05-26 Filip Pizlo <[email protected]>
Bogus uses of regexp matching should realize that they will OOM before they start swapping
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeList.json (201455 => 201456)
--- trunk/Source/_javascript_Core/bytecode/BytecodeList.json 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeList.json 2016-05-27 18:36:30 UTC (rev 201456)
@@ -61,6 +61,7 @@
{ "name" : "op_get_array_length", "length" : 9 },
{ "name" : "op_get_by_id", "length" : 9 },
{ "name" : "op_get_by_id_proto_load", "length" : 9 },
+ { "name" : "op_get_by_id_unset", "length" : 9 },
{ "name" : "op_get_by_id_with_this", "length" : 5 },
{ "name" : "op_get_by_val_with_this", "length" : 5 },
{ "name" : "op_try_get_by_id", "length" : 4 },
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeUseDef.h (201455 => 201456)
--- trunk/Source/_javascript_Core/bytecode/BytecodeUseDef.h 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeUseDef.h 2016-05-27 18:36:30 UTC (rev 201456)
@@ -159,6 +159,7 @@
case op_try_get_by_id:
case op_get_by_id:
case op_get_by_id_proto_load:
+ case op_get_by_id_unset:
case op_get_array_length:
case op_typeof:
case op_is_empty:
@@ -394,6 +395,7 @@
case op_try_get_by_id:
case op_get_by_id:
case op_get_by_id_proto_load:
+ case op_get_by_id_unset:
case op_get_by_id_with_this:
case op_get_by_val_with_this:
case op_get_array_length:
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (201455 => 201456)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-05-27 18:36:30 UTC (rev 201456)
@@ -349,6 +349,9 @@
case op_get_by_id_proto_load:
op = "get_by_id_proto_load";
break;
+ case op_get_by_id_unset:
+ op = "get_by_id_unset";
+ break;
case op_get_array_length:
op = "array_length";
break;
@@ -1119,6 +1122,7 @@
}
case op_get_by_id:
case op_get_by_id_proto_load:
+ case op_get_by_id_unset:
case op_get_array_length: {
printGetByIdOp(out, exec, location, it);
printGetByIdCacheStatus(out, exec, location, stubInfos);
@@ -2806,7 +2810,8 @@
Instruction* curInstruction = &instructions()[propertyAccessInstructions[i]];
switch (interpreter->getOpcodeID(curInstruction[0].u.opcode)) {
case op_get_by_id:
- case op_get_by_id_proto_load: {
+ case op_get_by_id_proto_load:
+ case op_get_by_id_unset: {
StructureID oldStructureID = curInstruction[4].u.structureID;
if (!oldStructureID || Heap::isMarked(m_vm->heap.structureIDTable().get(oldStructureID)))
break;
Modified: trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp (201455 => 201456)
--- trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp 2016-05-27 18:36:30 UTC (rev 201456)
@@ -78,9 +78,11 @@
Opcode opcode = instruction[0].u.opcode;
+ ASSERT(opcode == LLInt::getOpcode(op_get_array_length) || opcode == LLInt::getOpcode(op_try_get_by_id) || opcode == LLInt::getOpcode(op_get_by_id_proto_load) || opcode == LLInt::getOpcode(op_get_by_id) || opcode == LLInt::getOpcode(op_get_by_id_unset));
+
// FIXME: We should not just bail if we see a try_get_by_id or a get_by_id_proto_load.
// https://bugs.webkit.org/show_bug.cgi?id=158039
- if (opcode == LLInt::getOpcode(op_get_array_length) || opcode == LLInt::getOpcode(op_try_get_by_id) || opcode == LLInt::getOpcode(op_get_by_id_proto_load))
+ if (opcode != LLInt::getOpcode(op_get_by_id))
return GetByIdStatus(NoInformation, false);
StructureID structureID = instruction[4].u.structureID;
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (201455 => 201456)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-05-27 18:36:30 UTC (rev 201456)
@@ -4082,6 +4082,7 @@
case op_get_by_id:
case op_get_by_id_proto_load:
+ case op_get_by_id_unset:
case op_get_array_length: {
SpeculatedType prediction = getPrediction();
Modified: trunk/Source/_javascript_Core/dfg/DFGCapabilities.cpp (201455 => 201456)
--- trunk/Source/_javascript_Core/dfg/DFGCapabilities.cpp 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/dfg/DFGCapabilities.cpp 2016-05-27 18:36:30 UTC (rev 201456)
@@ -155,6 +155,7 @@
case op_try_get_by_id:
case op_get_by_id:
case op_get_by_id_proto_load:
+ case op_get_by_id_unset:
case op_get_by_id_with_this:
case op_get_by_val_with_this:
case op_get_array_length:
Modified: trunk/Source/_javascript_Core/jit/JIT.cpp (201455 => 201456)
--- trunk/Source/_javascript_Core/jit/JIT.cpp 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/jit/JIT.cpp 2016-05-27 18:36:30 UTC (rev 201456)
@@ -241,6 +241,7 @@
DEFINE_OP(op_try_get_by_id)
case op_get_array_length:
case op_get_by_id_proto_load:
+ case op_get_by_id_unset:
DEFINE_OP(op_get_by_id)
DEFINE_OP(op_get_by_id_with_this)
DEFINE_OP(op_get_by_val)
@@ -424,6 +425,7 @@
DEFINE_SLOWCASE_OP(op_try_get_by_id)
case op_get_array_length:
case op_get_by_id_proto_load:
+ case op_get_by_id_unset:
DEFINE_SLOWCASE_OP(op_get_by_id)
DEFINE_SLOWCASE_OP(op_get_by_val)
DEFINE_SLOWCASE_OP(op_instanceof)
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (201455 => 201456)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2016-05-27 18:36:30 UTC (rev 201456)
@@ -589,7 +589,11 @@
if (structure->typeInfo().prohibitsPropertyCaching() || structure->isDictionary())
return;
- ObjectPropertyConditionSet conditions = generateConditionsForPrototypePropertyHit(vm, codeBlock, exec, structure, slot.slotBase(), ident.impl());
+ ObjectPropertyConditionSet conditions;
+ if (slot.isUnset())
+ conditions = generateConditionsForPropertyMiss(vm, codeBlock, exec, structure, ident.impl());
+ else
+ conditions = generateConditionsForPrototypePropertyHit(vm, codeBlock, exec, structure, slot.slotBase(), ident.impl());
if (!conditions.isValid())
return;
@@ -604,16 +608,22 @@
offset = condition.condition().offset();
result.iterator->value.add(condition, pc)->install();
}
- ASSERT(offset != invalidOffset);
+ ASSERT((offset == invalidOffset) == slot.isUnset());
ConcurrentJITLocker locker(codeBlock->m_lock);
+ if (slot.isUnset()) {
+ pc[0].u.opcode = LLInt::getOpcode(op_get_by_id_unset);
+ pc[4].u.structureID = structure->id();
+ return;
+ }
+ ASSERT(slot.isValue());
+
pc[0].u.opcode = LLInt::getOpcode(op_get_by_id_proto_load);
pc[4].u.structureID = structure->id();
pc[5].u.operand = offset;
- // We know that this pointer will remain valid because it is the prototype of some structure, s,
- // watchpointed above. If any object with structure s were to change prototypes then the conditions
- // for this cache would fail and this value will never be used again.
+ // We know that this pointer will remain valid because it will be cleared by either a watchpoint fire or
+ // during GC when we clear the LLInt caches.
pc[6].u.pointer = slot.slotBase();
}
@@ -632,11 +642,11 @@
if (!LLINT_ALWAYS_ACCESS_SLOW
&& baseValue.isCell()
- && slot.isCacheableValue()) {
+ && slot.isCacheable()) {
JSCell* baseCell = baseValue.asCell();
Structure* structure = baseCell->structure();
- if (slot.slotBase() == baseValue) {
+ if (slot.isValue() && slot.slotBase() == baseValue) {
// Start out by clearing out the old cache.
pc[0].u.opcode = LLInt::getOpcode(op_get_by_id);
pc[4].u.pointer = nullptr; // old structure
@@ -653,7 +663,7 @@
pc[4].u.structureID = structure->id();
pc[5].u.operand = slot.cachedOffset();
}
- } else if (UNLIKELY(pc[7].u.operand)) {
+ } else if (UNLIKELY(pc[7].u.operand && (slot.isValue() || slot.isUnset()))) {
ASSERT(slot.slotBase() != baseValue);
if (!(--pc[7].u.operand))
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.h (201455 => 201456)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.h 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.h 2016-05-27 18:36:30 UTC (rev 201456)
@@ -71,7 +71,6 @@
LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_instanceof_custom);
LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_try_get_by_id);
LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_get_by_id);
-LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_get_by_id_proto_load);
LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_get_arguments_length);
LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_put_by_id);
LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_del_by_id);
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (201455 => 201456)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2016-05-27 18:36:30 UTC (rev 201456)
@@ -1337,8 +1337,9 @@
# opcode for own properties. We also allow for the cache to change anytime it fails,
# since ping-ponging is free. At best we get lucky and the get_by_id will continue
# to take fast path on the new cache. At worst we take slow path, which is what
-# we would have been doing anyway. For prototype properties, we will attempt to
-# convert opcode into a get_by_id_proto_load after a execution counter hits zero.
+# we would have been doing anyway. For prototype/unset properties, we will attempt to
+# convert opcode into a get_by_id_proto_load/get_by_id_unset, respectively, after an
+# execution counter hits zero.
_llint_op_get_by_id:
traceExecution()
@@ -1359,7 +1360,6 @@
dispatch(9)
-
_llint_op_get_by_id_proto_load:
traceExecution()
loadi 8[PC], t0
@@ -1380,6 +1380,23 @@
dispatch(9)
+_llint_op_get_by_id_unset:
+ traceExecution()
+ loadi 8[PC], t0
+ loadi 16[PC], t1
+ loadConstantOrVariablePayload(t0, CellTag, t3, .opGetByIdUnsetSlow)
+ bineq JSCell::m_structureID[t3], t1, .opGetByIdUnsetSlow
+ loadi 4[PC], t2
+ storei UndefinedTag, TagOffset[cfr, t2, 8]
+ storei 0, PayloadOffset[cfr, t2, 8]
+ valueProfile(UndefinedTag, 0, 32, t2)
+ dispatch(9)
+
+.opGetByIdUnsetSlow:
+ callSlowPath(_llint_slow_path_get_by_id)
+ dispatch(9)
+
+
_llint_op_get_array_length:
traceExecution()
loadi 8[PC], t0
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (201455 => 201456)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2016-05-27 18:35:49 UTC (rev 201455)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2016-05-27 18:36:30 UTC (rev 201456)
@@ -1252,6 +1252,23 @@
dispatch(9)
+_llint_op_get_by_id_unset:
+ traceExecution()
+ loadisFromInstruction(2, t0)
+ loadConstantOrVariableCell(t0, t3, .opGetByIdUnsetSlow)
+ loadi JSCell::m_structureID[t3], t1
+ loadisFromInstruction(4, t2)
+ bineq t2, t1, .opGetByIdUnsetSlow
+ loadisFromInstruction(1, t2)
+ storeq ValueUndefined, [cfr, t2, 8]
+ valueProfile(ValueUndefined, 8, t1)
+ dispatch(9)
+
+.opGetByIdUnsetSlow:
+ callSlowPath(_llint_slow_path_get_by_id)
+ dispatch(9)
+
+
_llint_op_get_array_length:
traceExecution()
loadisFromInstruction(2, t0)
