Skip to content

Commit

Permalink
[JSC] Clean up IC a bit
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=274350
rdar://128329113

Reviewed by Keith Miller.

1. Remove unused CodeBlock::getCallLinkInfoForBytecodeIndex. We are using JITCode's mechanism for this instead.
2. Set appropriate inlineCapacity for Vector<RefPtr<AccessCase>>.
3. Use Vector<Ref<AccessCase>> instead of Vector<RefPtr<AccessCase>>.

* Source/JavaScriptCore/bytecode/CodeBlock.cpp:
(JSC::CodeBlock::getCallLinkInfoForBytecodeIndex): Deleted.
* Source/JavaScriptCore/bytecode/CodeBlock.h:
* Source/JavaScriptCore/bytecode/InlineCacheCompiler.h:

Canonical link: https://commits.webkit.org/279003@main
  • Loading branch information
@Constellation
Constellation committed May 20, 2024
1 parent 90bbdbb commit bf1f3dd
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 94 deletions.
5 changes: 0 additions & 5 deletions Source/JavaScriptCore/bytecode/CallLinkInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,11 +381,6 @@ class BaselineCallLinkInfo final : public CallLinkInfo {
BytecodeIndex m_bytecodeIndex { };
};

inline CodeOrigin getCallLinkInfoCodeOrigin(CallLinkInfo& callLinkInfo)
{
return callLinkInfo.codeOrigin();
}

struct UnlinkedCallLinkInfo {
CodeLocationLabel<JSInternalPtrTag> doneLocation;

Expand Down
36 changes: 0 additions & 36 deletions Source/JavaScriptCore/bytecode/CodeBlock.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1751,42 +1751,6 @@ StructureStubInfo* CodeBlock::findStubInfo(CodeOrigin codeOrigin)
return result;
}

CallLinkInfo* CodeBlock::getCallLinkInfoForBytecodeIndex(const ConcurrentJSLocker&, BytecodeIndex index)
{
if (JITCode::couldBeInterpreted(jitType())) {
auto& instruction = instructions().at(index);
switch (instruction->opcodeID()) {
#define CASE(Op) \
case Op::opcodeID: \
return &instruction->as<Op>().metadata(this).m_callLinkInfo; \
break;

FOR_EACH_OPCODE_WITH_CALL_LINK_INFO(CASE)

#undef CASE
default:
break;
}
}

#if ENABLE(DFG_JIT)
if (JSC::JITCode::isOptimizingJIT(jitType())) {
DFG::CommonData* dfgCommon = m_jitCode->dfgCommon();
for (auto* callLinkInfo : dfgCommon->m_callLinkInfos) {
if (callLinkInfo->codeOrigin() == CodeOrigin(index))
return callLinkInfo;
}
if (auto* jitData = dfgJITData()) {
for (auto& callLinkInfo : jitData->callLinkInfos()) {
if (callLinkInfo.codeOrigin() == CodeOrigin(index))
return &callLinkInfo;
}
}
}
#endif
return nullptr;
}

void CodeBlock::resetBaselineJITData()
{
RELEASE_ASSERT(!JITCode::isJIT(jitType()));
Expand Down
5 changes: 0 additions & 5 deletions Source/JavaScriptCore/bytecode/CodeBlock.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,6 @@ class CodeBlock : public JSCell {
// O(n) operation. Use getICStatusMap() unless you really only intend to get one stub info.
StructureStubInfo* findStubInfo(CodeOrigin);

// This is a slow function call used primarily for compiling OSR exits in the case
// that there had been inlining. Chances are if you want to use this, you're really
// looking for a CallLinkInfoMap to amortize the cost of calling this.
CallLinkInfo* getCallLinkInfoForBytecodeIndex(const ConcurrentJSLocker&, BytecodeIndex);

const JITCodeMap& jitCodeMap();

std::optional<CodeOrigin> findPC(void* pc);
Expand Down
34 changes: 17 additions & 17 deletions Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4004,15 +4004,15 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
// to be unmutated. For sure, we want it to hang onto any data structures that may be referenced
// from the code of the current stub (aka previous).
Vector<WatchpointSet*, 8> additionalWatchpointSets;
Vector<RefPtr<AccessCase>, 16> cases;
Vector<Ref<AccessCase>, 16> cases;
cases.reserveInitialCapacity(poly.m_list.size());
unsigned srcIndex = 0;
for (auto& someCase : poly.m_list) {
[&] () {
if (!someCase->couldStillSucceed())
return;

auto sets = collectAdditionalWatchpoints(vm(), *someCase);
auto sets = collectAdditionalWatchpoints(vm(), someCase.get());
for (auto* set : sets) {
if (!set->isStillValid())
return;
Expand All @@ -4031,7 +4031,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
// If we can generate a binary switch, then A->canReplace(B) == B->canReplace(A). So,
// it doesn't matter that we only do the check in one direction.
for (unsigned j = srcIndex + 1; j < poly.m_list.size(); ++j) {
if (poly.m_list[j]->canReplace(*someCase))
if (poly.m_list[j]->canReplace(someCase.get()))
return;
}

Expand Down Expand Up @@ -4345,7 +4345,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
bool allGuardedByStructureCheck = true;
if (!m_stubInfo->hasConstantIdentifier)
allGuardedByStructureCheck = false;
FixedVector<RefPtr<AccessCase>> keys(WTFMove(cases));
FixedVector<Ref<AccessCase>> keys(WTFMove(cases));
m_callLinkInfos.resize(keys.size());
Vector<JSCell*> cellsToMark;
for (auto& entry : keys) {
Expand Down Expand Up @@ -4422,7 +4422,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
fallThrough.link(&jit);
fallThrough.shrink(0);
if (keys[i]->requiresInt32PropertyCheck())
generateWithGuard(i, *keys[i], fallThrough);
generateWithGuard(i, keys[i].get(), fallThrough);
}

if (needsStringPropertyCheck || needsSymbolPropertyCheck || acceptValueProperty) {
Expand Down Expand Up @@ -4455,7 +4455,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
fallThrough.link(&jit);
fallThrough.shrink(0);
if (keys[i]->requiresIdentifierNameMatch() && !keys[i]->uid()->isSymbol())
generateWithGuard(i, *keys[i], fallThrough);
generateWithGuard(i, keys[i].get(), fallThrough);
}

if (needsSymbolPropertyCheck || acceptValueProperty) {
Expand Down Expand Up @@ -4484,7 +4484,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
fallThrough.link(&jit);
fallThrough.shrink(0);
if (keys[i]->requiresIdentifierNameMatch() && keys[i]->uid()->isSymbol())
generateWithGuard(i, *keys[i], fallThrough);
generateWithGuard(i, keys[i].get(), fallThrough);
}

if (acceptValueProperty) {
Expand All @@ -4501,7 +4501,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
fallThrough.link(&jit);
fallThrough.shrink(0);
if (!keys[i]->requiresIdentifierNameMatch() && !keys[i]->requiresInt32PropertyCheck())
generateWithGuard(i, *keys[i], fallThrough);
generateWithGuard(i, keys[i].get(), fallThrough);
}
}
} else {
Expand All @@ -4510,7 +4510,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
for (unsigned i = keys.size(); i--;) {
fallThrough.link(&jit);
fallThrough.shrink(0);
generateWithGuard(i, *keys[i], fallThrough);
generateWithGuard(i, keys[i].get(), fallThrough);
}
}

Expand All @@ -4528,7 +4528,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL

BinarySwitch binarySwitch(m_scratchGPR, caseValues.span(), BinarySwitch::Int32);
while (binarySwitch.advance(jit))
generate(binarySwitch.caseIndex(), *keys[binarySwitch.caseIndex()]);
generate(binarySwitch.caseIndex(), keys[binarySwitch.caseIndex()].get());
m_failAndRepatch.append(binarySwitch.fallThrough());
}

Expand Down Expand Up @@ -4681,7 +4681,7 @@ AccessGenerationResult InlineCacheCompiler::regenerate(const GCSafeConcurrentJSL
PolymorphicAccess::PolymorphicAccess() = default;
PolymorphicAccess::~PolymorphicAccess() = default;

AccessGenerationResult PolymorphicAccess::addCases(const GCSafeConcurrentJSLocker&, VM& vm, CodeBlock*, StructureStubInfo& stubInfo, Vector<RefPtr<AccessCase>, 2> originalCasesToAdd)
AccessGenerationResult PolymorphicAccess::addCases(const GCSafeConcurrentJSLocker&, VM& vm, CodeBlock*, StructureStubInfo& stubInfo, ListType&& originalCasesToAdd)
{
SuperSamplerScope superSamplerScope(false);

Expand All @@ -4697,14 +4697,14 @@ AccessGenerationResult PolymorphicAccess::addCases(const GCSafeConcurrentJSLocke
// add more things after failure.

// First ensure that the originalCasesToAdd doesn't contain duplicates.
Vector<RefPtr<AccessCase>> casesToAdd;
ListType casesToAdd;
for (unsigned i = 0; i < originalCasesToAdd.size(); ++i) {
RefPtr<AccessCase> myCase = WTFMove(originalCasesToAdd[i]);
Ref<AccessCase> myCase = WTFMove(originalCasesToAdd[i]);

// Add it only if it is not replaced by the subsequent cases in the list.
bool found = false;
for (unsigned j = i + 1; j < originalCasesToAdd.size(); ++j) {
if (originalCasesToAdd[j]->canReplace(*myCase)) {
if (originalCasesToAdd[j]->canReplace(myCase.get())) {
found = true;
break;
}
Expand Down Expand Up @@ -4763,7 +4763,7 @@ AccessGenerationResult PolymorphicAccess::addCases(const GCSafeConcurrentJSLocke
// Now add things to the new list. Note that at this point, we will still have old cases that
// may be replaced by the new ones. That's fine. We will sort that out when we regenerate.
for (auto& caseToAdd : casesToAdd) {
collectAdditionalWatchpoints(vm, *caseToAdd);
collectAdditionalWatchpoints(vm, caseToAdd.get());
m_list.append(WTFMove(caseToAdd));
}

Expand All @@ -4775,7 +4775,7 @@ AccessGenerationResult PolymorphicAccess::addCases(const GCSafeConcurrentJSLocke
AccessGenerationResult PolymorphicAccess::addCase(
const GCSafeConcurrentJSLocker& locker, VM& vm, CodeBlock* codeBlock, StructureStubInfo& stubInfo, Ref<AccessCase> newAccess)
{
Vector<RefPtr<AccessCase>, 2> newAccesses;
ListType newAccesses;
newAccesses.append(WTFMove(newAccess));
return addCases(locker, vm, codeBlock, stubInfo, WTFMove(newAccesses));
}
Expand Down Expand Up @@ -4813,7 +4813,7 @@ void PolymorphicAccess::dump(PrintStream& out) const
out.print(RawPointer(this), ":["_s);
CommaPrinter comma;
for (auto& entry : m_list)
out.print(comma, *entry);
out.print(comma, entry.get());
out.print("]"_s);
}

Expand Down
11 changes: 5 additions & 6 deletions Source/JavaScriptCore/bytecode/InlineCacheCompiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,22 @@ class PolymorphicAccess {
public:
friend class InlineCacheCompiler;

using ListType = Vector<Ref<AccessCase>, 16>;

PolymorphicAccess();
~PolymorphicAccess();

// When this fails (returns GaveUp), this will leave the old stub intact but you should not try
// to call this method again for that PolymorphicAccess instance.
AccessGenerationResult addCases(
const GCSafeConcurrentJSLocker&, VM&, CodeBlock*, StructureStubInfo&, Vector<RefPtr<AccessCase>, 2>);
AccessGenerationResult addCases(const GCSafeConcurrentJSLocker&, VM&, CodeBlock*, StructureStubInfo&, ListType&&);

AccessGenerationResult addCase(
const GCSafeConcurrentJSLocker&, VM&, CodeBlock*, StructureStubInfo&, Ref<AccessCase>);

bool isEmpty() const { return m_list.isEmpty(); }
unsigned size() const { return m_list.size(); }
const AccessCase& at(unsigned i) const { return *m_list[i]; }
const AccessCase& operator[](unsigned i) const { return *m_list[i]; }
const AccessCase& at(unsigned i) const { return m_list[i].get(); }
const AccessCase& operator[](unsigned i) const { return m_list[i].get(); }

DECLARE_VISIT_AGGREGATE;

Expand All @@ -152,8 +153,6 @@ class PolymorphicAccess {
friend class CodeBlock;
friend class InlineCacheCompiler;

typedef Vector<RefPtr<AccessCase>, 2> ListType;

ListType m_list;
RefPtr<PolymorphicAccessJITStubRoutine> m_stubRoutine;
};
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/bytecode/SharedJITStubSet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class SharedJITStubSet {
if (aCases.size() != bCases.size())
return false;
for (unsigned index = 0; index < bCases.size(); ++index) {
if (!AccessCase::canBeShared(*aCases[index], *bCases[index]))
if (!AccessCase::canBeShared(aCases[index].get(), bCases[index].get()))
return false;
}
return true;
Expand All @@ -109,7 +109,7 @@ class SharedJITStubSet {
};

StructureStubInfoKey m_stubInfoKey;
std::span<const RefPtr<AccessCase>> m_cases;
std::span<const Ref<AccessCase>> m_cases;
};

struct PointerTranslator {
Expand Down
19 changes: 9 additions & 10 deletions Source/JavaScriptCore/bytecode/StructureStubInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,16 @@ AccessGenerationResult StructureStubInfo::addAccessCase(
}
} else {
std::unique_ptr<PolymorphicAccess> access = makeUnique<PolymorphicAccess>();

Vector<RefPtr<AccessCase>, 2> accessCases;

auto previousCase = AccessCase::fromStructureStubInfo(vm, codeBlock, ident, *this);
if (previousCase)
accessCases.append(WTFMove(previousCase));

accessCases.append(WTFMove(accessCase));


PolymorphicAccess::ListType accessCases;

if (auto previousCase = AccessCase::fromStructureStubInfo(vm, codeBlock, ident, *this))
accessCases.append(previousCase.releaseNonNull());

accessCases.append(accessCase.releaseNonNull());

result = access->addCases(locker, vm, codeBlock, *this, WTFMove(accessCases));

if (StructureStubInfoInternal::verbose)
dataLog("Created stub, result: ", result, "\n");

Expand Down
10 changes: 5 additions & 5 deletions Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ bool GCAwareJITStubRoutine::removeDeadOwners(VM& vm)
return false;
}

PolymorphicAccessJITStubRoutine::PolymorphicAccessJITStubRoutine(Type type, const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code, VM& vm, FixedVector<RefPtr<AccessCase>>&& cases, FixedVector<StructureID>&& weakStructures, JSCell* owner)
PolymorphicAccessJITStubRoutine::PolymorphicAccessJITStubRoutine(Type type, const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code, VM& vm, FixedVector<Ref<AccessCase>>&& cases, FixedVector<StructureID>&& weakStructures, JSCell* owner)
: GCAwareJITStubRoutine(type, code, owner)
, m_vm(vm)
, m_cases(WTFMove(cases))
Expand Down Expand Up @@ -139,7 +139,7 @@ void PolymorphicAccessJITStubRoutine::invalidate()
}
}

unsigned PolymorphicAccessJITStubRoutine::computeHash(std::span<const RefPtr<AccessCase>> cases)
unsigned PolymorphicAccessJITStubRoutine::computeHash(std::span<const Ref<AccessCase>> cases)
{
Hasher hasher;
for (auto& key : cases)
Expand All @@ -153,7 +153,7 @@ void PolymorphicAccessJITStubRoutine::addedToSharedJITStubSet()
}

MarkingGCAwareJITStubRoutine::MarkingGCAwareJITStubRoutine(
Type type, const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code, VM& vm, FixedVector<RefPtr<AccessCase>>&& cases, FixedVector<StructureID>&& weakStructures, JSCell* owner,
Type type, const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code, VM& vm, FixedVector<Ref<AccessCase>>&& cases, FixedVector<StructureID>&& weakStructures, JSCell* owner,
const Vector<JSCell*>& cells, Vector<std::unique_ptr<OptimizingCallLinkInfo>, 16>&& callLinkInfos)
: PolymorphicAccessJITStubRoutine(type, code, vm, WTFMove(cases), WTFMove(weakStructures), owner)
, m_cells(cells.size())
Expand Down Expand Up @@ -195,7 +195,7 @@ CallLinkInfo* MarkingGCAwareJITStubRoutine::callLinkInfoAtImpl(const ConcurrentJ
return nullptr;
}

GCAwareJITStubRoutineWithExceptionHandler::GCAwareJITStubRoutineWithExceptionHandler(const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code, VM& vm, FixedVector<RefPtr<AccessCase>>&& cases, FixedVector<StructureID>&& weakStructures, JSCell* owner, const Vector<JSCell*>& cells, Vector<std::unique_ptr<OptimizingCallLinkInfo>, 16>&& callLinkInfos,
GCAwareJITStubRoutineWithExceptionHandler::GCAwareJITStubRoutineWithExceptionHandler(const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code, VM& vm, FixedVector<Ref<AccessCase>>&& cases, FixedVector<StructureID>&& weakStructures, JSCell* owner, const Vector<JSCell*>& cells, Vector<std::unique_ptr<OptimizingCallLinkInfo>, 16>&& callLinkInfos,
CodeBlock* codeBlockForExceptionHandlers, DisposableCallSiteIndex exceptionHandlerCallSiteIndex)
: MarkingGCAwareJITStubRoutine(JITStubRoutine::Type::GCAwareJITStubRoutineWithExceptionHandlerType, code, vm, WTFMove(cases), WTFMove(weakStructures), owner, cells, WTFMove(callLinkInfos))
, m_codeBlockWithExceptionHandler(codeBlockForExceptionHandlers)
Expand Down Expand Up @@ -235,7 +235,7 @@ void GCAwareJITStubRoutineWithExceptionHandler::observeZeroRefCountImpl()

Ref<PolymorphicAccessJITStubRoutine> createICJITStubRoutine(
const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code,
FixedVector<RefPtr<AccessCase>>&& cases,
FixedVector<Ref<AccessCase>>&& cases,
FixedVector<StructureID>&& weakStructures,
VM& vm,
JSCell* owner,
Expand Down

0 comments on commit bf1f3dd

Please sign in to comment.