Skip to content

Commit d41bc43

Browse files
ConstellationConstellation
committed
[JSC] Filter out GetByStatus with constant identifier
https://bugs.webkit.org/show_bug.cgi?id=294705 rdar://153788181 Reviewed by Yijia Huang. This patch prepares for the further optimizations. 1. We filter GetByStatus out with constant identifier. It leverages GetById instead of GetByVal if possible later. 2. Remove m_seenIdentifiers. Just using normal IC for fallback. * Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj: * Source/JavaScriptCore/SaferCPPExpectations/MemoryUnsafeCastCheckerExpectations: * Source/JavaScriptCore/bytecode/BytecodeList.rb: * Source/JavaScriptCore/bytecode/GetByStatus.cpp: (JSC::GetByStatus::filterById): * Source/JavaScriptCore/bytecode/GetByStatus.h: * Source/JavaScriptCore/bytecode/GetByValHistory.h: Removed. * Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp: (JSC::InlineCacheCompiler::compile): (JSC::InlineCacheCompiler::compileOneAccessCaseHandler): * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::parseBlock): * Source/JavaScriptCore/generator/DSL.rb: * Source/JavaScriptCore/jit/JITPropertyAccess.cpp: (JSC::JIT::emit_op_get_by_val): * Source/JavaScriptCore/llint/LLIntSlowPaths.cpp: (JSC::LLInt::LLINT_SLOW_PATH_DECL): Canonical link: https://commits.webkit.org/296493@main
1 parent 0922e18 commit d41bc43

11 files changed

Lines changed: 68 additions & 148 deletions

File tree

‎Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -955,7 +955,6 @@
955955
4BDF3004289324AC00AE1DE3 /* AssemblyComments.h in Headers */ = {isa = PBXBuildFile; fileRef = 4BDF3002289324AC00AE1DE3 /* AssemblyComments.h */; settings = {ATTRIBUTES = (Private, ); }; };
956956
4BE92D442898522400FA48E1 /* LowLevelInterpreter.asm in Sources */ = {isa = PBXBuildFile; fileRef = 86A054461556451B00445157 /* LowLevelInterpreter.asm */; };
957957
4BE92D452898522400FA48E1 /* LowLevelInterpreter.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F4680C714BBB16900BFE272 /* LowLevelInterpreter.cpp */; };
958-
520D99F12388CC81000509A3 /* GetByValHistory.h in Headers */ = {isa = PBXBuildFile; fileRef = 520D99F02388CC78000509A3 /* GetByValHistory.h */; };
959958
521131F71F82BF14007CCEEE /* PolyProtoAccessChain.h in Headers */ = {isa = PBXBuildFile; fileRef = 521131F61F82BF11007CCEEE /* PolyProtoAccessChain.h */; settings = {ATTRIBUTES = (Private, ); }; };
960959
521322461ECBCE8200F65615 /* WebAssemblyFunctionBase.h in Headers */ = {isa = PBXBuildFile; fileRef = 521322441ECBCE8200F65615 /* WebAssemblyFunctionBase.h */; };
961960
522927D5235FD0B9005CB169 /* GCMemoryOperations.h in Headers */ = {isa = PBXBuildFile; fileRef = 5272987B235FC8BA005C982C /* GCMemoryOperations.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -4058,7 +4057,6 @@
40584057
4CE978E385A8498199052153 /* ModuleNamespaceAccessCase.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ModuleNamespaceAccessCase.h; sourceTree = "<group>"; };
40594058
51F0EB6105C86C6B00E6DF1B /* Foundation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Foundation.framework; path = /System/Library/Frameworks/Foundation.framework; sourceTree = "<absolute>"; };
40604059
51F0EC0705C86C9A00E6DF1B /* libobjc.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; name = libobjc.dylib; path = /usr/lib/libobjc.dylib; sourceTree = "<absolute>"; };
4061-
520D99F02388CC78000509A3 /* GetByValHistory.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = GetByValHistory.h; sourceTree = "<group>"; };
40624060
521131F51F82BF11007CCEEE /* PolyProtoAccessChain.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = PolyProtoAccessChain.cpp; sourceTree = "<group>"; };
40634061
521131F61F82BF11007CCEEE /* PolyProtoAccessChain.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = PolyProtoAccessChain.h; sourceTree = "<group>"; };
40644062
521322431ECBCE8200F65615 /* WebAssemblyFunctionBase.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = WebAssemblyFunctionBase.cpp; path = js/WebAssemblyFunctionBase.cpp; sourceTree = "<group>"; };
@@ -9658,7 +9656,6 @@
96589656
1498CAD5214BF36D00710879 /* GetByIdMetadata.h */,
96599657
0F93329514CA7DC10085F3C6 /* GetByStatus.cpp */,
96609658
0F93329614CA7DC10085F3C6 /* GetByStatus.h */,
9661-
520D99F02388CC78000509A3 /* GetByValHistory.h */,
96629659
0F0332C118B01763005F979A /* GetByVariant.cpp */,
96639660
0F0332C218B01763005F979A /* GetByVariant.h */,
96649661
14AD91081DCA92940014F9FE /* GlobalCodeBlock.h */,
@@ -11064,7 +11061,6 @@
1106411061
0F2B66E117B6B5AB00A7AE3F /* GenericTypedArrayViewInlines.h in Headers */,
1106511062
1498CAD6214BF36D00710879 /* GetByIdMetadata.h in Headers */,
1106611063
0F9332A014CA7DCD0085F3C6 /* GetByStatus.h in Headers */,
11067-
520D99F12388CC81000509A3 /* GetByValHistory.h in Headers */,
1106811064
0F0332C418B01763005F979A /* GetByVariant.h in Headers */,
1106911065
7964656A1B952FF0003059EE /* GetPutInfo.h in Headers */,
1107011066
84323E7A25D5EC3B00F97776 /* GetterSetter.h in Headers */,

‎Source/JavaScriptCore/SaferCPPExpectations/MemoryUnsafeCastCheckerExpectations‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ interpreter/StackVisitor.cpp
5353
jit/JITOperations.cpp
5454
jit/JITThunks.cpp
5555
jsc.cpp
56-
llint/LLIntSlowPaths.cpp
5756
parser/ASTBuilder.h
5857
parser/NodesAnalyzeModule.cpp
5958
runtime/ArrayStorage.h

‎Source/JavaScriptCore/bytecode/BytecodeList.rb‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
:EnumeratorMetadata,
3434
:GetByIdMode,
3535
:GetByIdModeMetadata,
36-
:GetByValHistory,
3736
:GetPutInfo,
3837
:IndexingType,
3938
:IterationModeMetadata,
@@ -580,59 +579,57 @@
580579
arrayAllocationProfile: ArrayAllocationProfile,
581580
}
582581

583-
op :get_by_val_with_this,
582+
op :put_private_name,
584583
args: {
585-
dst: VirtualRegister,
586584
base: VirtualRegister,
587-
thisValue: VirtualRegister,
588585
property: VirtualRegister,
589-
valueProfile: unsigned,
586+
value: VirtualRegister,
587+
putKind: PrivateFieldPutKind,
590588
},
591589
metadata: {
592-
arrayProfile: ArrayProfile,
593-
seenIdentifiers: GetByValHistory,
590+
property: WriteBarrier[JSCell],
591+
oldStructureID: StructureID,
592+
offset: unsigned,
593+
newStructureID: StructureID,
594594
}
595595

596-
op :get_by_val,
596+
op :get_private_name,
597597
args: {
598598
dst: VirtualRegister,
599599
base: VirtualRegister,
600600
property: VirtualRegister,
601601
valueProfile: unsigned,
602602
},
603603
metadata: {
604-
arrayProfile: ArrayProfile,
605-
seenIdentifiers: GetByValHistory,
604+
structureID: StructureID,
605+
offset: unsigned,
606+
property: WriteBarrier[JSCell],
606607
}
607608

608-
op :put_private_name,
609+
# Alignment: 4
610+
op :get_by_val_with_this,
609611
args: {
612+
dst: VirtualRegister,
610613
base: VirtualRegister,
614+
thisValue: VirtualRegister,
611615
property: VirtualRegister,
612-
value: VirtualRegister,
613-
putKind: PrivateFieldPutKind,
616+
valueProfile: unsigned,
614617
},
615618
metadata: {
616-
property: WriteBarrier[JSCell],
617-
oldStructureID: StructureID,
618-
offset: unsigned,
619-
newStructureID: StructureID,
619+
arrayProfile: ArrayProfile,
620620
}
621621

622-
op :get_private_name,
622+
op :get_by_val,
623623
args: {
624624
dst: VirtualRegister,
625625
base: VirtualRegister,
626626
property: VirtualRegister,
627627
valueProfile: unsigned,
628628
},
629629
metadata: {
630-
structureID: StructureID,
631-
offset: unsigned,
632-
property: WriteBarrier[JSCell],
630+
arrayProfile: ArrayProfile,
633631
}
634632

635-
# Alignment: 4
636633
op :put_by_val,
637634
args: {
638635
base: VirtualRegister,

‎Source/JavaScriptCore/bytecode/GetByStatus.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,24 @@ CacheableIdentifier GetByStatus::singleIdentifier() const
675675
return singleIdentifierForICStatus(m_variants);
676676
}
677677

678+
void GetByStatus::filterById(UniquedStringImpl* uid)
679+
{
680+
if (m_state != Simple)
681+
return;
682+
683+
if (m_variants.isEmpty())
684+
return;
685+
686+
auto filtered = m_variants;
687+
filtered.removeAllMatching(
688+
[&] (auto& variant) -> bool {
689+
return variant.identifier() != uid;
690+
});
691+
if (filtered.isEmpty())
692+
return;
693+
m_variants = WTFMove(filtered);
694+
}
695+
678696
#if ENABLE(JIT)
679697

680698
CacheType GetByStatus::preferredCacheType() const

‎Source/JavaScriptCore/bytecode/GetByStatus.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ class GetByStatus final {
127127

128128
// Attempts to reduce the set of variants to fit the given structure set. This may be approximate.
129129
void filter(const StructureSet&);
130+
void filterById(UniquedStringImpl*);
130131

131132
JSModuleNamespaceObject* moduleNamespaceObject() const { return m_moduleNamespaceData->m_moduleNamespaceObject; }
132133
JSModuleEnvironment* moduleEnvironment() const { return m_moduleNamespaceData->m_moduleEnvironment; }

‎Source/JavaScriptCore/bytecode/GetByValHistory.h‎

Lines changed: 0 additions & 74 deletions
This file was deleted.

‎Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5113,7 +5113,7 @@ AccessGenerationResult InlineCacheCompiler::compile(const GCSafeConcurrentJSLock
51135113

51145114
dataLogLnIf(InlineCacheCompilerInternal::verbose, FullCodeOrigin(codeBlock, m_stubInfo.codeOrigin), ": Generating polymorphic access stub for ", listDump(keys));
51155115

5116-
MacroAssemblerCodeRef<JITStubRoutinePtrTag> code = FINALIZE_CODE_FOR(codeBlock, linkBuffer, JITStubRoutinePtrTag, categoryName(m_stubInfo.accessType), "%s", toCString("Access stub for ", *codeBlock, " ", m_stubInfo.codeOrigin, "with start: ", m_stubInfo.startLocation, " with return point ", successLabel, ": ", listDump(keys)).data());
5116+
MacroAssemblerCodeRef<JITStubRoutinePtrTag> code = FINALIZE_CODE_FOR(codeBlock, linkBuffer, JITStubRoutinePtrTag, categoryName(m_stubInfo.accessType), "%s", toCString("Access stub for ", *codeBlock, " ", m_stubInfo.codeOrigin, " with start: ", m_stubInfo.startLocation, " with return point ", successLabel, ": ", listDump(keys)).data());
51175117

51185118
CodeBlock* owner = codeBlock;
51195119
FixedVector<StructureID> weakStructures(WTFMove(m_weakStructures));
@@ -7578,7 +7578,7 @@ AccessGenerationResult InlineCacheCompiler::compileOneAccessCaseHandler(const Ve
75787578
auto keys = FixedVector<Ref<AccessCase>> { Ref { accessCase } };
75797579
dataLogLnIf(InlineCacheCompilerInternal::verbose, FullCodeOrigin(codeBlock, m_stubInfo.codeOrigin), ": Generating polymorphic access stub for ", listDump(keys));
75807580

7581-
MacroAssemblerCodeRef<JITStubRoutinePtrTag> code = FINALIZE_CODE_FOR(codeBlock, linkBuffer, JITStubRoutinePtrTag, categoryName(m_stubInfo.accessType), "%s", toCString("Access stub for ", *codeBlock, " ", m_stubInfo.codeOrigin, "with start: ", m_stubInfo.startLocation, ": ", listDump(keys)).data());
7581+
MacroAssemblerCodeRef<JITStubRoutinePtrTag> code = FINALIZE_CODE_FOR(codeBlock, linkBuffer, JITStubRoutinePtrTag, categoryName(m_stubInfo.accessType), "%s", toCString("Access stub for ", *codeBlock, " ", m_stubInfo.codeOrigin, " with start: ", m_stubInfo.startLocation, ": ", listDump(keys)).data());
75827582

75837583
if (statelessType) {
75847584
auto stub = createPreCompiledICJITStubRoutine(WTFMove(code), vm, codeBlock);

‎Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp‎

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7714,11 +7714,20 @@ void ByteCodeParser::parseBlock(unsigned limit)
77147714

77157715
Node* base = get(bytecode.m_base);
77167716
Node* property = get(bytecode.m_property);
7717-
bool shouldCompileAsGetById = false;
77187717
GetByStatus getByStatus = GetByStatus::computeFor(m_inlineStackTop->m_profiledBlock, m_inlineStackTop->m_baselineMap, m_icContextStack, currentCodeOrigin());
77197718
unsigned identifierNumber = 0;
7720-
77217719
CacheableIdentifier identifier;
7720+
7721+
if (auto* string = property->dynamicCastConstant<JSString*>()) {
7722+
auto* impl = string->tryGetValueImpl();
7723+
if (impl && impl->isAtom() && !parseIndex(*const_cast<StringImpl*>(impl))) {
7724+
auto* uid = static_cast<UniquedStringImpl*>(impl);
7725+
identifierNumber = m_graph.identifiers().ensure(uid);
7726+
m_graph.freezeStrong(string);
7727+
getByStatus.filterById(uid);
7728+
}
7729+
}
7730+
77227731
if (!m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadIdent)
77237732
&& !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadType)
77247733
&& !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadConstantValue)) {
@@ -7738,31 +7747,28 @@ void ByteCodeParser::parseBlock(unsigned limit)
77387747
addToGraph(CheckIdent, OpInfo(uid), property);
77397748
} else
77407749
addToGraph(CheckIdent, OpInfo(uid), property);
7741-
shouldCompileAsGetById = true;
7750+
handleGetById(bytecode.m_dst, prediction, base, identifier, identifierNumber, getByStatus, AccessType::GetById, nextOpcodeIndex());
7751+
NEXT_OPCODE(op_get_by_val);
77427752
}
77437753
}
77447754

7745-
if (shouldCompileAsGetById)
7746-
handleGetById(bytecode.m_dst, prediction, base, identifier, identifierNumber, getByStatus, AccessType::GetById, nextOpcodeIndex());
7747-
else {
7748-
if (getByStatus.isProxyObject()) {
7749-
if (handleIndexedProxyObjectLoad(bytecode.m_dst, prediction, base, property, getByStatus, nextOpcodeIndex())) {
7750-
NEXT_OPCODE(op_get_by_val);
7751-
}
7755+
if (getByStatus.isProxyObject()) {
7756+
if (handleIndexedProxyObjectLoad(bytecode.m_dst, prediction, base, property, getByStatus, nextOpcodeIndex())) {
7757+
NEXT_OPCODE(op_get_by_val);
77527758
}
7753-
ArrayMode arrayMode = getArrayMode(bytecode.metadata(codeBlock).m_arrayProfile, Array::Read);
7754-
// FIXME: We could consider making this not vararg, since it only uses three child
7755-
// slots.
7756-
// https://bugs.webkit.org/show_bug.cgi?id=184192
7757-
addVarArgChild(base);
7758-
addVarArgChild(property);
7759-
addVarArgChild(nullptr); // Leave room for property storage.
7760-
Node* getByVal = addToGraph(Node::VarArg, getByStatus.isMegamorphic() ? GetByValMegamorphic : GetByVal, OpInfo(arrayMode.asWord()), OpInfo(prediction));
7761-
m_exitOK = false; // GetByVal must be treated as if it clobbers exit state, since FixupPhase may make it generic.
7762-
set(bytecode.m_dst, getByVal);
7763-
if (!getByStatus.isMegamorphic() && getByStatus.observedStructureStubInfoSlowPath())
7764-
m_graph.m_slowGetByVal.add(getByVal);
77657759
}
7760+
ArrayMode arrayMode = getArrayMode(bytecode.metadata(codeBlock).m_arrayProfile, Array::Read);
7761+
// FIXME: We could consider making this not vararg, since it only uses three child
7762+
// slots.
7763+
// https://bugs.webkit.org/show_bug.cgi?id=184192
7764+
addVarArgChild(base);
7765+
addVarArgChild(property);
7766+
addVarArgChild(nullptr); // Leave room for property storage.
7767+
Node* getByVal = addToGraph(Node::VarArg, getByStatus.isMegamorphic() ? GetByValMegamorphic : GetByVal, OpInfo(arrayMode.asWord()), OpInfo(prediction));
7768+
m_exitOK = false; // GetByVal must be treated as if it clobbers exit state, since FixupPhase may make it generic.
7769+
set(bytecode.m_dst, getByVal);
7770+
if (!getByStatus.isMegamorphic() && getByStatus.observedStructureStubInfoSlowPath())
7771+
m_graph.m_slowGetByVal.add(getByVal);
77667772

77677773
NEXT_OPCODE(op_get_by_val);
77687774
}

‎Source/JavaScriptCore/generator/DSL.rb‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,6 @@ def self.write_bytecode_structs(bytecode_list, bytecode_structs_filename)
217217
#include "BytecodeDumper.h"
218218
#include "Fits.h"
219219
#include "GetByIdMetadata.h"
220-
#include "GetByValHistory.h"
221220
#include "Instruction.h"
222221
#include "IterationModeMetadata.h"
223222
#include "JSPropertyNameEnumerator.h"

‎Source/JavaScriptCore/jit/JITPropertyAccess.cpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,6 @@ void JIT::emit_op_get_by_val(const JSInstruction* currentInstruction)
7373
if (isOperandConstantInt(property))
7474
stubInfo->propertyIsInt32 = true;
7575

76-
if (bytecode.metadata(m_profiledCodeBlock).m_seenIdentifiers.count() > Options::getByValICMaxNumberOfIdentifiers())
77-
stubInfo->canBeMegamorphic = true;
78-
7976
emitJumpSlowCaseIfNotJSCell(baseJSR, base);
8077
emitArrayProfilingSiteWithCellAndProfile(baseJSR.payloadGPR(), profileGPR, scratch1GPR);
8178

0 commit comments

Comments
 (0)