Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (286634 => 286635)
--- trunk/Source/_javascript_Core/ChangeLog 2021-12-08 01:35:15 UTC (rev 286634)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-12-08 01:43:22 UTC (rev 286635)
@@ -1,3 +1,32 @@
+2021-12-07 Ross Kirsling <ross.kirsl...@sony.com>
+
+ [JSC] Add LLInt IC for try_get_by_id of own cacheable value
+ https://bugs.webkit.org/show_bug.cgi?id=233830
+
+ Reviewed by Yusuke Suzuki.
+
+ This patch adds an LLInt IC for the "own cacheable value" path of try_get_by_id;
+ this is the simplest case and basically the same as get_by_id_direct.
+
+ Performance is neutral with JIT enabled as well as on current uses of try_get_by_id in JSC
+ (e.g. hasObservableSideEffectsForRegexpSplit), but microbenchmarks of try_get_by_id itself see a 2x speedup:
+
+ Before After
+
+ try-get-by-id-polymorphic 123.8361+-0.4562 ^ 61.7586+-0.3770 ^ definitely 2.0052x faster
+ try-get-by-id-basic 124.4437+-0.6091 ^ 61.0340+-0.1924 ^ definitely 2.0389x faster
+
+ <geometric> 124.1207+-0.3130 ^ 61.3865+-0.2019 ^ definitely 2.0220x faster
+
+
+ * bytecode/BytecodeList.rb:
+ * bytecode/CodeBlock.cpp:
+ * bytecode/GetByStatus.cpp:
+ * llint/LLIntSlowPaths.cpp:
+ * llint/LowLevelInterpreter.asm:
+ * llint/LowLevelInterpreter32_64.asm:
+ * llint/LowLevelInterpreter64.asm:
+
2021-12-07 Commit Queue <commit-qu...@webkit.org>
Unreviewed, reverting r286502 and r286580.
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeList.rb (286634 => 286635)
--- trunk/Source/_javascript_Core/bytecode/BytecodeList.rb 2021-12-08 01:35:15 UTC (rev 286634)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeList.rb 2021-12-08 01:43:22 UTC (rev 286635)
@@ -523,6 +523,8 @@
},
metadata: {
profile: ValueProfile,
+ structureID: StructureID,
+ offset: unsigned,
}
op :put_by_id,
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (286634 => 286635)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2021-12-08 01:35:15 UTC (rev 286634)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2021-12-08 01:43:22 UTC (rev 286635)
@@ -1386,11 +1386,20 @@
clearIfNeeded(metadata.m_modeMetadata, "get by id"_s);
});
+ m_metadata->forEach<OpTryGetById>([&] (auto& metadata) {
+ StructureID oldStructureID = metadata.m_structureID;
+ if (!oldStructureID || vm.heap.isMarked(oldStructureID.decode()))
+ return;
+ dataLogLnIf(Options::verboseOSR(), "Clearing try_get_by_id LLInt property access.");
+ metadata.m_structureID = StructureID();
+ metadata.m_offset = 0;
+ });
+
m_metadata->forEach<OpGetByIdDirect>([&] (auto& metadata) {
StructureID oldStructureID = metadata.m_structureID;
if (!oldStructureID || vm.heap.isMarked(oldStructureID.decode()))
return;
- dataLogLnIf(Options::verboseOSR(), "Clearing LLInt property access.");
+ dataLogLnIf(Options::verboseOSR(), "Clearing get_by_id_direct LLInt property access.");
metadata.m_structureID = StructureID();
metadata.m_offset = 0;
});
@@ -1461,7 +1470,7 @@
&& (!brand || vm.heap.isMarked(brand)))
return;
- dataLogLnIf(Options::verboseOSR(), "Clearing LLInt set_private_brand transition.");
+ dataLogLnIf(Options::verboseOSR(), "Clearing LLInt check_private_brand transition.");
metadata.m_structureID = StructureID();
metadata.m_brand.clear();
});
Modified: trunk/Source/_javascript_Core/bytecode/GetByStatus.cpp (286634 => 286635)
--- trunk/Source/_javascript_Core/bytecode/GetByStatus.cpp 2021-12-08 01:35:15 UTC (rev 286634)
+++ trunk/Source/_javascript_Core/bytecode/GetByStatus.cpp 2021-12-08 01:43:22 UTC (rev 286635)
@@ -70,15 +70,15 @@
identifier = &(profiledBlock->identifier(instruction->as<OpGetById>().m_property));
break;
}
+
+ case op_try_get_by_id:
+ structureID = instruction->as<OpTryGetById>().metadata(profiledBlock).m_structureID;
+ identifier = &(profiledBlock->identifier(instruction->as<OpTryGetById>().m_property));
+ break;
case op_get_by_id_direct:
structureID = instruction->as<OpGetByIdDirect>().metadata(profiledBlock).m_structureID;
identifier = &(profiledBlock->identifier(instruction->as<OpGetByIdDirect>().m_property));
break;
- case op_try_get_by_id: {
- // FIXME: We should not just bail if we see a try_get_by_id.
- // https://bugs.webkit.org/show_bug.cgi?id=158039
- return GetByStatus(NoInformation, false);
- }
case op_get_by_val:
return GetByStatus(NoInformation, false);
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (286634 => 286635)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2021-12-08 01:35:15 UTC (rev 286634)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2021-12-08 01:43:22 UTC (rev 286635)
@@ -664,6 +664,42 @@
baseValue.getPropertySlot(globalObject, ident, slot);
JSValue result = slot.getPureResult();
+ if (!LLINT_ALWAYS_ACCESS_SLOW && slot.isCacheable() && !slot.isUnset()) {
+ ASSERT(!slot.isTaintedByOpaqueObject());
+ ASSERT(baseValue.isCell());
+
+ auto& metadata = bytecode.metadata(codeBlock);
+ {
+ StructureID oldStructureID = metadata.m_structureID;
+ if (oldStructureID) {
+ Structure* a = oldStructureID.decode();
+ Structure* b = baseValue.asCell()->structure(vm);
+
+ if (Structure::shouldConvertToPolyProto(a, b)) {
+ ASSERT(a->rareData()->sharedPolyProtoWatchpoint().get() == b->rareData()->sharedPolyProtoWatchpoint().get());
+ a->rareData()->sharedPolyProtoWatchpoint()->invalidate(vm, StringFireDetail("Detected poly proto opportunity."));
+ }
+ }
+ }
+
+ JSCell* baseCell = baseValue.asCell();
+ Structure* structure = baseCell->structure(vm);
+ if (slot.isValue() && slot.slotBase() == baseValue) {
+ // Start out by clearing out the old cache.
+ metadata.m_structureID = StructureID();
+ metadata.m_offset = 0;
+
+ if (structure->propertyAccessesAreCacheable() && !structure->needImpurePropertyWatchpoint()) {
+ {
+ ConcurrentJSLocker locker(codeBlock->m_lock);
+ metadata.m_structureID = structure->id();
+ metadata.m_offset = slot.cachedOffset();
+ }
+ vm.writeBarrier(codeBlock);
+ }
+ }
+ }
+
LLINT_RETURN_PROFILED(result);
}
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm (286634 => 286635)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm 2021-12-08 01:35:15 UTC (rev 286634)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm 2021-12-08 01:43:22 UTC (rev 286635)
@@ -2180,7 +2180,6 @@
llintSlowPathOp(super_sampler_begin)
llintSlowPathOp(super_sampler_end)
llintSlowPathOp(throw)
-llintSlowPathOp(try_get_by_id)
llintSlowPathOp(get_by_id_with_this)
llintOp(op_switch_string, unused, macro (unused, unused, unused)
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (286634 => 286635)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2021-12-08 01:35:15 UTC (rev 286634)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2021-12-08 01:43:22 UTC (rev 286635)
@@ -1421,6 +1421,22 @@
# convert opcode into a get_by_id_proto_load/get_by_id_unset, respectively, after an
# execution counter hits zero.
+llintOpWithMetadata(op_try_get_by_id, OpTryGetById, macro (size, get, dispatch, metadata, return)
+ metadata(t5, t0)
+ get(m_base, t0)
+ loadi OpTryGetById::Metadata::m_structureID[t5], t1
+ loadConstantOrVariablePayload(size, t0, CellTag, t3, .opTryGetByIdSlow)
+ loadi OpTryGetById::Metadata::m_offset[t5], t2
+ bineq JSCell::m_structureID[t3], t1, .opTryGetByIdSlow
+ loadPropertyAtVariableOffset(t2, t3, t0, t1)
+ valueProfile(OpTryGetById, m_profile, t5, t0, t1)
+ return(t0, t1)
+
+.opTryGetByIdSlow:
+ callSlowPath(_llint_slow_path_try_get_by_id)
+ dispatch()
+end)
+
llintOpWithMetadata(op_get_by_id_direct, OpGetByIdDirect, macro (size, get, dispatch, metadata, return)
metadata(t5, t0)
get(m_base, t0)
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (286634 => 286635)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2021-12-08 01:35:15 UTC (rev 286634)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2021-12-08 01:43:22 UTC (rev 286635)
@@ -1533,6 +1533,23 @@
end
+llintOpWithMetadata(op_try_get_by_id, OpTryGetById, macro (size, get, dispatch, metadata, return)
+ metadata(t2, t0)
+ get(m_base, t0)
+ loadConstantOrVariableCell(size, t0, t3, .opTryGetByIdSlow)
+ loadi JSCell::m_structureID[t3], t1
+ loadi OpTryGetById::Metadata::m_structureID[t2], t0
+ bineq t0, t1, .opTryGetByIdSlow
+ loadi OpTryGetById::Metadata::m_offset[t2], t1
+ loadPropertyAtVariableOffset(t1, t3, t0)
+ valueProfile(OpTryGetById, m_profile, t2, t0)
+ return(t0)
+
+.opTryGetByIdSlow:
+ callSlowPath(_llint_slow_path_try_get_by_id)
+ dispatch()
+end)
+
llintOpWithMetadata(op_get_by_id_direct, OpGetByIdDirect, macro (size, get, dispatch, metadata, return)
metadata(t2, t0)
get(m_base, t0)
