Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JSC] Expand InByVal ICs to cover integer indices as well #17704

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
327 changes: 234 additions & 93 deletions Source/JavaScriptCore/bytecode/AccessCase.cpp

Large diffs are not rendered by default.

26 changes: 26 additions & 0 deletions Source/JavaScriptCore/bytecode/AccessCase.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,32 @@ DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(AccessCase);
macro(IndexedResizableTypedArrayUint32Store) \
macro(IndexedResizableTypedArrayFloat32Store) \
macro(IndexedResizableTypedArrayFloat64Store) \
macro(IndexedInt32InHit) \
macro(IndexedDoubleInHit) \
macro(IndexedContiguousInHit) \
macro(IndexedArrayStorageInHit) \
macro(IndexedScopedArgumentsInHit) \
macro(IndexedDirectArgumentsInHit) \
macro(IndexedTypedArrayInt8InHit) \
macro(IndexedTypedArrayUint8InHit) \
macro(IndexedTypedArrayUint8ClampedInHit) \
macro(IndexedTypedArrayInt16InHit) \
macro(IndexedTypedArrayUint16InHit) \
macro(IndexedTypedArrayInt32InHit) \
macro(IndexedTypedArrayUint32InHit) \
macro(IndexedTypedArrayFloat32InHit) \
macro(IndexedTypedArrayFloat64InHit) \
macro(IndexedResizableTypedArrayInt8InHit) \
macro(IndexedResizableTypedArrayUint8InHit) \
macro(IndexedResizableTypedArrayUint8ClampedInHit) \
macro(IndexedResizableTypedArrayInt16InHit) \
macro(IndexedResizableTypedArrayUint16InHit) \
macro(IndexedResizableTypedArrayInt32InHit) \
macro(IndexedResizableTypedArrayUint32InHit) \
macro(IndexedResizableTypedArrayFloat32InHit) \
macro(IndexedResizableTypedArrayFloat64InHit) \
macro(IndexedStringInHit) \
macro(IndexedNoIndexingInMiss) \


class AccessCase : public ThreadSafeRefCounted<AccessCase> {
Expand Down
7 changes: 7 additions & 0 deletions Source/JavaScriptCore/bytecode/InByStatus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,13 @@ InByStatus InByStatus::computeForStubInfoWithoutExitSiteFeedback(const Concurren
if (access.usesPolyProto())
return InByStatus(TakesSlowPath);

if (!access.requiresIdentifierNameMatch()) {
// FIXME: We could use this for indexed loads in the future. This is pretty solid profiling
// information, and probably better than ArrayProfile when it's available.
// https://bugs.webkit.org/show_bug.cgi?id=204215
return InByStatus(TakesSlowPath);
}

Structure* structure = access.structure();
if (!structure) {
// The null structure cases arise due to array.length. We have no way of creating a
Expand Down
543 changes: 453 additions & 90 deletions Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp

Large diffs are not rendered by default.

130 changes: 130 additions & 0 deletions Source/JavaScriptCore/bytecode/Repatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1664,6 +1664,136 @@ static InlineCacheAction tryCacheInstanceOf(
return result.shouldGiveUpNow() ? GiveUpOnCache : RetryCacheLater;
}

static InlineCacheAction tryCacheArrayInByVal(JSGlobalObject* globalObject, CodeBlock* codeBlock, JSValue baseValue, JSValue index, StructureStubInfo& stubInfo)
{
ASSERT(baseValue.isCell());

if (!index.isInt32())
return RetryCacheLater;

VM& vm = globalObject->vm();
AccessGenerationResult result;

{
GCSafeConcurrentJSLocker locker(codeBlock->m_lock, globalObject->vm());

JSCell* base = baseValue.asCell();

RefPtr<AccessCase> newCase;
AccessCase::AccessType accessType = AccessCase::IndexedInt32InHit;
if (base->type() == DirectArgumentsType)
accessType = AccessCase::IndexedDirectArgumentsInHit;
else if (base->type() == ScopedArgumentsType)
accessType = AccessCase::IndexedScopedArgumentsInHit;
else if (base->type() == StringType)
accessType = AccessCase::IndexedStringInHit;
else if (isTypedView(base->type())) {
auto* typedArray = jsCast<JSArrayBufferView*>(base);
#if USE(JSVALUE32_64)
if (typedArray->isResizableOrGrowableShared())
return GiveUpOnCache;
#endif
switch (typedArray->type()) {
case Int8ArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayInt8InHit : AccessCase::IndexedTypedArrayInt8InHit;
break;
case Uint8ArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayUint8InHit : AccessCase::IndexedTypedArrayUint8InHit;
break;
case Uint8ClampedArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayUint8ClampedInHit : AccessCase::IndexedTypedArrayUint8ClampedInHit;
break;
case Int16ArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayInt16InHit : AccessCase::IndexedTypedArrayInt16InHit;
break;
case Uint16ArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayUint16InHit : AccessCase::IndexedTypedArrayUint16InHit;
break;
case Int32ArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayInt32InHit : AccessCase::IndexedTypedArrayInt32InHit;
break;
case Uint32ArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayUint32InHit : AccessCase::IndexedTypedArrayUint32InHit;
break;
case Float32ArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayFloat32InHit : AccessCase::IndexedTypedArrayFloat32InHit;
break;
case Float64ArrayType:
accessType = typedArray->isResizableOrGrowableShared() ? AccessCase::IndexedResizableTypedArrayFloat64InHit : AccessCase::IndexedTypedArrayFloat64InHit;
break;
// FIXME: Optimize BigInt64Array / BigUint64Array in IC
// https://bugs.webkit.org/show_bug.cgi?id=221183
case BigInt64ArrayType:
case BigUint64ArrayType:
return GiveUpOnCache;
default:
RELEASE_ASSERT_NOT_REACHED();
}
} else {
IndexingType indexingShape = base->indexingType() & IndexingShapeMask;
switch (indexingShape) {
case Int32Shape:
accessType = AccessCase::IndexedInt32InHit;
break;
case DoubleShape:
ASSERT(Options::allowDoubleShape());
accessType = AccessCase::IndexedDoubleInHit;
break;
case ContiguousShape:
accessType = AccessCase::IndexedContiguousInHit;
break;
case ArrayStorageShape:
accessType = AccessCase::IndexedArrayStorageInHit;
break;
case NoIndexingShape: {
if (!base->isObject())
return GiveUpOnCache;

if (base->structure()->mayInterceptIndexedAccesses() || base->structure()->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero())
return GiveUpOnCache;

// FIXME: prepareChainForCaching is conservative. We should have another function which only cares about information related to this IC.
auto cacheStatus = prepareChainForCaching(globalObject, base, nullptr, nullptr);
if (!cacheStatus)
return GiveUpOnCache;

if (cacheStatus->usesPolyProto)
return GiveUpOnCache;

Structure* headStructure = base->structure();
ObjectPropertyConditionSet conditionSet = generateConditionsForIndexedMiss(vm, codeBlock, globalObject, headStructure);
if (!conditionSet.isValid())
return GiveUpOnCache;

newCase = AccessCase::create(vm, codeBlock, AccessCase::IndexedNoIndexingInMiss, nullptr, invalidOffset, headStructure, conditionSet);
break;
}
default:
return GiveUpOnCache;
}
}

if (!newCase)
newCase = AccessCase::create(vm, codeBlock, accessType, nullptr);

result = stubInfo.addAccessCase(locker, globalObject, codeBlock, ECMAMode::strict(), nullptr, newCase.releaseNonNull());

if (result.generatedSomeCode()) {
RELEASE_ASSERT(result.code());
InlineAccess::rewireStubAsJumpInAccessNotUsingInlineAccess(codeBlock, stubInfo, CodeLocationLabel<JITStubRoutinePtrTag>(result.code()));
}
}

fireWatchpointsAndClearStubIfNeeded(vm, stubInfo, codeBlock, result);
return result.shouldGiveUpNow() ? GiveUpOnCache : RetryCacheLater;
}

void repatchArrayInByVal(JSGlobalObject* globalObject, CodeBlock* codeBlock, JSValue base, JSValue index, StructureStubInfo& stubInfo, InByKind kind)
{
if (tryCacheArrayInByVal(globalObject, codeBlock, base, index, stubInfo) == GiveUpOnCache)
repatchSlowPathCall(codeBlock, stubInfo, appropriateInByGaveUpFunction(kind));
}

void repatchInstanceOf(
JSGlobalObject* globalObject, CodeBlock* codeBlock, JSValue valueValue, JSValue prototypeValue, StructureStubInfo& stubInfo,
bool wasFound)
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/bytecode/Repatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ void repatchGetBy(JSGlobalObject*, CodeBlock*, JSValue, CacheableIdentifier, con
void repatchArrayPutByVal(JSGlobalObject*, CodeBlock*, JSValue base, JSValue index, StructureStubInfo&, PutByKind);
void repatchPutBy(JSGlobalObject*, CodeBlock*, JSValue, Structure*, CacheableIdentifier, const PutPropertySlot&, StructureStubInfo&, PutByKind);
void repatchDeleteBy(JSGlobalObject*, CodeBlock*, DeletePropertySlot&, JSValue, Structure*, CacheableIdentifier, StructureStubInfo&, DelByKind, ECMAMode);
void repatchArrayInByVal(JSGlobalObject*, CodeBlock*, JSValue base, JSValue index, StructureStubInfo&, InByKind);
void repatchInBy(JSGlobalObject*, CodeBlock*, JSObject*, CacheableIdentifier, bool wasFound, const PropertySlot&, StructureStubInfo&, InByKind);
void repatchHasPrivateBrand(JSGlobalObject*, CodeBlock*, JSObject*, CacheableIdentifier, bool wasFound, StructureStubInfo&);
void repatchCheckPrivateBrand(JSGlobalObject*, CodeBlock*, JSObject*, CacheableIdentifier, StructureStubInfo&);
Expand Down
74 changes: 70 additions & 4 deletions Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5666,11 +5666,77 @@ void SpeculativeJIT::compile(Node* node)
}

case HasIndexedProperty: {
SpeculateStrictInt32Operand index(this, m_graph.varArgChild(node, 1));
GPRTemporary result(this, Reuse, index);
ArrayMode mode = node->arrayMode();
switch (mode.type()) {
case Array::Int32:
case Array::Contiguous:
case Array::Double:
case Array::ArrayStorage: {
SpeculateStrictInt32Operand index(this, m_graph.varArgChild(node, 1));
GPRTemporary result(this, Reuse, index);

compileHasIndexedProperty(node, operationHasIndexedProperty, scopedLambda<std::tuple<GPRReg, GPRReg>()>([&] { return std::make_pair(index.gpr(), result.gpr()); }));
unblessedBooleanResult(result.gpr(), node);
break;
}
default: {
SpeculateCellOperand base(this, m_graph.varArgChild(node, 0));
JSValueOperand key(this, m_graph.varArgChild(node, 1), ManualOperandSpeculation);
JSValueRegsTemporary result(this, Reuse, key);
std::optional<GPRTemporary> stubInfoTemp;

GPRReg stubInfoGPR = InvalidGPRReg;
if (m_graph.m_plan.isUnlinked()) {
stubInfoTemp.emplace(this);
stubInfoGPR = stubInfoTemp->gpr();
}
GPRReg baseGPR = base.gpr();
JSValueRegs keyRegs = key.jsValueRegs();
JSValueRegs resultRegs = result.regs();

if (m_graph.varArgChild(node, 0).useKind() == ObjectUse)
speculateObject(m_graph.varArgChild(node, 0), baseGPR);
speculate(node, m_graph.varArgChild(node, 1));

JumpList slowCases;

CodeOrigin codeOrigin = node->origin.semantic;
CallSiteIndex callSite = recordCallSiteAndGenerateExceptionHandlingOSRExitIfNeeded(codeOrigin, m_stream.size());
RegisterSetBuilder usedRegisters = this->usedRegisters();
auto [ stubInfo, stubInfoConstant ] = addStructureStubInfo();
JITInByValGenerator gen(
codeBlock(), stubInfo, JITType::DFGJIT, codeOrigin, callSite, AccessType::InByVal, usedRegisters,
JSValueRegs::payloadOnly(baseGPR), keyRegs, resultRegs, InvalidGPRReg, stubInfoGPR);

std::visit([&](auto* stubInfo) {
stubInfo->propertyIsInt32 = true;
stubInfo->isEnumerator = false;
}, stubInfo);

std::unique_ptr<SlowPathGenerator> slowPath;
if (m_graph.m_plan.isUnlinked()) {
gen.generateDFGDataICFastPath(*this, stubInfoConstant.index(), stubInfoGPR);
ASSERT(!gen.stubInfo());
slowPath = slowPathICCall(
slowCases, this, stubInfoConstant, stubInfoGPR, Address(stubInfoGPR, StructureStubInfo::offsetOfSlowOperation()), operationInByValOptimize,
NeedToSpill, ExceptionCheckRequirement::CheckNeeded,
resultRegs, CellValue(baseGPR), keyRegs, LinkableConstant::globalObject(*this, node), stubInfoGPR, nullptr);
} else {
gen.generateFastPath(*this);
slowCases.append(gen.slowPathJump());
slowPath = slowPathCall(
slowCases, this, operationInByValOptimize,
NeedToSpill, ExceptionCheckRequirement::CheckNeeded,
resultRegs, CellValue(baseGPR), keyRegs, LinkableConstant::globalObject(*this, node), TrustedImmPtr(gen.stubInfo()), nullptr);
}

compileHasIndexedProperty(node, operationHasIndexedProperty, scopedLambda<std::tuple<GPRReg, GPRReg>()>([&] { return std::make_pair(index.gpr(), result.gpr()); }));
unblessedBooleanResult(result.gpr(), node);
addInByVal(gen, slowPath.get());
addSlowPathGenerator(WTFMove(slowPath));

blessedBooleanResult(resultRegs.payloadGPR(), node);
break;
}
}
break;
}

Expand Down
24 changes: 22 additions & 2 deletions Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14833,8 +14833,28 @@ IGNORE_CLANG_WARNINGS_END

void compileHasIndexedProperty()
{
LValue index = lowInt32(m_graph.varArgChild(m_node, 1));
setBoolean(compileHasIndexedPropertyImpl(index, operationHasIndexedProperty));
ArrayMode mode = m_node->arrayMode();
switch (mode.type()) {
case Array::Int32:
case Array::Contiguous:
case Array::Double:
case Array::ArrayStorage: {
LValue index = lowInt32(m_graph.varArgChild(m_node, 1));
setBoolean(compileHasIndexedPropertyImpl(index, operationHasIndexedProperty));
break;
}
default: {
LValue base = nullptr;
if (m_graph.varArgChild(m_node, 0).useKind() == ObjectUse)
base = lowObject(m_graph.varArgChild(m_node, 0));
else
base = lowCell(m_graph.varArgChild(m_node, 0));
LValue property = lowJSValue(m_graph.varArgChild(m_node, 1), ManualOperandSpeculation);
speculate(m_graph.varArgChild(m_node, 1));
compileInBy<AccessType::InByVal>(base, property);
break;
}
}
}

void compileGetPropertyEnumerator()
Expand Down
27 changes: 18 additions & 9 deletions Source/JavaScriptCore/jit/JITOperations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ JSC_DEFINE_JIT_OPERATION(operationInByIdOptimize, EncodedJSValue, (EncodedJSValu
return JSValue::encode(jsBoolean(found));
}

JSC_DEFINE_JIT_OPERATION(operationInByValOptimize, EncodedJSValue, (EncodedJSValue encodedBase, EncodedJSValue encodedKey, JSGlobalObject* globalObject, StructureStubInfo* stubInfo, ArrayProfile* arrayProfile))
JSC_DEFINE_JIT_OPERATION(operationInByValOptimize, EncodedJSValue, (EncodedJSValue encodedBase, EncodedJSValue encodedKey, JSGlobalObject* globalObject, StructureStubInfo* stubInfo, ArrayProfile* profile))
{
SuperSamplerScope superSamplerScope(false);

Expand All @@ -681,21 +681,30 @@ JSC_DEFINE_JIT_OPERATION(operationInByValOptimize, EncodedJSValue, (EncodedJSVal
auto scope = DECLARE_THROW_SCOPE(vm);

JSValue baseValue = JSValue::decode(encodedBase);
JSValue key = JSValue::decode(encodedKey);

if (!baseValue.isObject()) {
throwException(globalObject, scope, createInvalidInParameterError(globalObject, baseValue));
return encodedJSValue();
return { };
}

JSObject* baseObject = asObject(baseValue);
if (arrayProfile)
arrayProfile->observeStructure(baseObject->structure());

JSValue key = JSValue::decode(encodedKey);
uint32_t i;
if (key.getUInt32(i)) {
// FIXME: InByVal should have inline caching for integer indices too, as GetByVal does.
// https://bugs.webkit.org/show_bug.cgi?id=226619
if (arrayProfile)
arrayProfile->observeIndexedRead(baseObject, i);
if (profile)
profile->observeIndexedRead(baseObject, i);

CodeBlock* codeBlock = callFrame->codeBlock();
Structure* structure = baseValue.asCell()->structure();
if (stubInfo->considerRepatchingCacheGeneric(vm, codeBlock, structure)) {
if (profile) {
ConcurrentJSLocker locker(codeBlock->m_lock);
profile->computeUpdatedPrediction(locker, codeBlock, structure);
}
repatchArrayInByVal(globalObject, codeBlock, baseValue, key, *stubInfo, InByKind::ByVal);
}

RELEASE_AND_RETURN(scope, JSValue::encode(jsBoolean(baseObject->hasProperty(globalObject, i))));
}

Expand Down
3 changes: 0 additions & 3 deletions Source/JavaScriptCore/runtime/CommonSlowPaths.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ inline bool opInByVal(JSGlobalObject* globalObject, JSValue baseVal, JSValue pro
}

JSObject* baseObj = asObject(baseVal);
if (arrayProfile)
arrayProfile->observeStructure(baseObj->structure());

uint32_t i;
if (propName.getUInt32(i)) {
if (arrayProfile)
Expand Down