Skip to content

Commit

Permalink
[JSC] Clean up Float32Array access a bit more
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=268055
rdar://116791798

Reviewed by Justin Michaud.

This patch further optimizes Float32Array access.

1. We forget removing some unnecessary code in FTLLowerDFGToB3 from 273389@main. It does not have any effect, but it is simply wasteful.
2. In most of cases, we already emit CheckStructure for Float32Array. So we can leverage this structure information to ensure that this
   is never resizable / growable array. We use AI to remove this check, this contributes largely since we remove unnecessary load from
   Float32Array object!
3. We add more DFGMayExit cases for CompareStrictEq since this is common. We can observe this pattern in the benchmark script, and this
   makes it possible to wipe some of unnecessary exit cases, which allows us to remove some MovHint & its ValueRep.

                               ToT                     Patched

    segmentation         91.7487+-0.2857     ^     83.8224+-0.2430        ^ definitely 1.0946x faster

* Source/JavaScriptCore/dfg/DFGGraph.cpp:
(JSC::DFG::Graph::isNeverResizableOrGrowableSharedTypedArrayIncludingDataView):
* Source/JavaScriptCore/dfg/DFGGraph.h:
* Source/JavaScriptCore/dfg/DFGMayExit.cpp:
* Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::jumpForTypedArrayOutOfBounds):
* Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compileGetTypedArrayLengthAsInt52):
(JSC::DFG::SpeculativeJIT::compileGetTypedArrayByteOffsetAsInt52):
(JSC::DFG::SpeculativeJIT::compile):
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::emitGetTypedArrayByteOffsetExceptSettingResult):
(JSC::FTL::DFG::LowerDFGToB3::typedArrayLength):
(JSC::FTL::DFG::LowerDFGToB3::compileGetArrayLength):
(JSC::FTL::DFG::LowerDFGToB3::compileGetTypedArrayLengthAsInt52):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):

Canonical link: https://commits.webkit.org/273481@main
  • Loading branch information
Constellation committed Jan 25, 2024
1 parent a011563 commit eebb374
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 36 deletions.
18 changes: 18 additions & 0 deletions Source/JavaScriptCore/dfg/DFGGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1956,6 +1956,24 @@ bool Graph::canDoFastSpread(Node* node, const AbstractValue& value)
return allGood;
}

bool Graph::isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(const AbstractValue& value)
{
auto& structureSet = value.m_structure;
if (!structureSet.isFinite())
return false;

if (structureSet.isClear())
return false;

bool allAreNonResizable = true;
structureSet.forEach(
[&](RegisteredStructure structure) {
if (isResizableOrGrowableSharedTypedArrayIncludingDataView(structure->classInfoForCells()))
allAreNonResizable = false;
});
return allAreNonResizable;
}

void Graph::clearCPSCFGData()
{
m_cpsNaturalLoops = nullptr;
Expand Down
2 changes: 2 additions & 0 deletions Source/JavaScriptCore/dfg/DFGGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,8 @@ class Graph final : public virtual Scannable {

void freeDFGIRAfterLowering();

bool isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(const AbstractValue&);

const BoyerMooreHorspoolTable<uint8_t>* tryAddStringSearchTable8(const String&);

StackCheck m_stackChecker;
Expand Down
5 changes: 4 additions & 1 deletion Source/JavaScriptCore/dfg/DFGMayExit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,11 @@ ExitMode mayExitImpl(Graph& graph, Node* node, StateType& state)
break;
return Exits;

case CompareEq:
case CompareStrictEq:
if (node->isBinaryUseKind(MiscUse, UntypedUse) || node->isBinaryUseKind(UntypedUse, MiscUse))
break;
FALLTHROUGH;
case CompareEq:
case CompareLess:
case CompareLessEq:
case CompareGreater:
Expand Down
15 changes: 10 additions & 5 deletions Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3836,8 +3836,8 @@ JITCompiler::Jump SpeculativeJIT::jumpForTypedArrayOutOfBounds(Node* node, GPRRe
{
if (node->op() == PutByValAlias)
return Jump();
JSArrayBufferView* view = m_graph.tryGetFoldableView(
m_state.forNode(m_graph.child(node, 0)).m_value, node->arrayMode());
Edge& edge = m_graph.child(node, 0);
JSArrayBufferView* view = m_graph.tryGetFoldableView(m_state.forNode(edge).m_value, node->arrayMode());
if (view && !view->isResizableOrGrowableShared()) {
size_t length = view->length();
Node* indexNode = m_graph.child(node, 1).node();
Expand Down Expand Up @@ -3868,7 +3868,9 @@ JITCompiler::Jump SpeculativeJIT::jumpForTypedArrayOutOfBounds(Node* node, GPRRe
UNUSED_PARAM(scratch2GPR);
#endif

speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
if (!m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(edge)))
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));

#if USE(LARGE_TYPED_ARRAYS)
signExtend32ToPtr(indexGPR, scratchGPR);
return branch64(
Expand Down Expand Up @@ -8844,7 +8846,9 @@ void SpeculativeJIT::compileGetTypedArrayByteOffset(Node* node)
GPRReg baseGPR = base.gpr();
GPRReg resultGPR = result.gpr();

speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));

if (!m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(node->child1())))
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));

#if USE(LARGE_TYPED_ARRAYS)
load64(Address(baseGPR, JSArrayBufferView::offsetOfByteOffset()), resultGPR);
Expand Down Expand Up @@ -9138,7 +9142,8 @@ void SpeculativeJIT::compileGetArrayLength(Node* node)
GPRReg baseGPR = base.gpr();
GPRReg resultGPR = result.gpr();

speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
if (!m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(node->child1())))
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
#if USE(LARGE_TYPED_ARRAYS)
load64(Address(baseGPR, JSArrayBufferView::offsetOfLength()), resultGPR);
speculationCheck(ExitKind::Overflow, JSValueSource(), nullptr, branch64(Above, resultGPR, TrustedImm64(std::numeric_limits<int32_t>::max())));
Expand Down
13 changes: 9 additions & 4 deletions Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2957,7 +2957,9 @@ void SpeculativeJIT::compileGetTypedArrayLengthAsInt52(Node* node)
GPRTemporary result(this, Reuse, base);
GPRReg baseGPR = base.gpr();
GPRReg resultGPR = result.gpr();
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));

if (!m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(node->child1())))
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
load64(Address(baseGPR, JSArrayBufferView::offsetOfLength()), resultGPR);
static_assert(MAX_ARRAY_BUFFER_SIZE < (1ull << 52), "there is a risk that the size of a typed array won't fit in an Int52");
strictInt52Result(resultGPR, node);
Expand Down Expand Up @@ -2993,7 +2995,8 @@ void SpeculativeJIT::compileGetTypedArrayByteOffsetAsInt52(Node* node)
GPRReg baseGPR = base.gpr();
GPRReg resultGPR = result.gpr();

speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
if (!m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(node->child1())))
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(baseGPR), node, branchTest8(NonZero, Address(baseGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));

load64(Address(baseGPR, JSArrayBufferView::offsetOfByteOffset()), resultGPR);
strictInt52Result(resultGPR, node);
Expand Down Expand Up @@ -5858,7 +5861,8 @@ void SpeculativeJIT::compile(Node* node)
if (data.isResizable)
loadTypedArrayLength(dataViewGPR, t1, t2, t1, TypeDataView);
else {
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(dataViewGPR), node, branchTest8(NonZero, Address(dataViewGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
if (!m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(node->child1())))
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(dataViewGPR), node, branchTest8(NonZero, Address(dataViewGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
#if USE(LARGE_TYPED_ARRAYS)
load64(Address(dataViewGPR, JSArrayBufferView::offsetOfLength()), t1);
#else
Expand Down Expand Up @@ -6075,7 +6079,8 @@ void SpeculativeJIT::compile(Node* node)
if (data.isResizable)
loadTypedArrayLength(dataViewGPR, t1, t2, t1, TypeDataView);
else {
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(dataViewGPR), node, branchTest8(NonZero, Address(dataViewGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
if (!m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(m_graph.varArgChild(node, 0))))
speculationCheck(UnexpectedResizableArrayBufferView, JSValueSource::unboxedCell(dataViewGPR), node, branchTest8(NonZero, Address(dataViewGPR, JSArrayBufferView::offsetOfMode()), TrustedImm32(isResizableOrGrowableSharedMode)));
#if USE(LARGE_TYPED_ARRAYS)
load64(Address(dataViewGPR, JSArrayBufferView::offsetOfLength()), t1);
#else
Expand Down
35 changes: 9 additions & 26 deletions Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5484,7 +5484,8 @@ class LowerDFGToB3 {
return patchpoint;
}

speculate(UnexpectedResizableArrayBufferView, jsValueValue(base), m_node, m_out.testNonZero32(m_out.load8ZeroExt32(base, m_heaps.JSArrayBufferView_mode), m_out.constInt32(isResizableOrGrowableSharedMode)));
if (!m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(m_node->child1())))
speculate(UnexpectedResizableArrayBufferView, jsValueValue(base), m_node, m_out.testNonZero32(m_out.load8ZeroExt32(base, m_heaps.JSArrayBufferView_mode), m_out.constInt32(isResizableOrGrowableSharedMode)));
#if USE(LARGE_TYPED_ARRAYS)
return m_out.load64(base, m_heaps.JSArrayBufferView_byteOffset);
#else
Expand Down Expand Up @@ -5643,7 +5644,7 @@ IGNORE_CLANG_WARNINGS_END
setJSValue(m_out.loadPtr(moduleRecord, m_heaps.WebAssemblyModuleRecord_exportsObject));
}

LValue typedArrayLength(LValue base, bool acceptResizable, std::optional<TypedArrayType> typedArrayType)
LValue typedArrayLength(LValue base, bool acceptResizable, std::optional<TypedArrayType> typedArrayType, Edge edge = Edge())
{
if (acceptResizable) {
#if USE(LARGE_TYPED_ARRAYS)
Expand All @@ -5669,7 +5670,8 @@ IGNORE_CLANG_WARNINGS_END
return patchpoint;
}

speculate(UnexpectedResizableArrayBufferView, jsValueValue(base), m_node, m_out.testNonZero32(m_out.load8ZeroExt32(base, m_heaps.JSArrayBufferView_mode), m_out.constInt32(isResizableOrGrowableSharedMode)));
if (!edge || !m_graph.isNeverResizableOrGrowableSharedTypedArrayIncludingDataView(m_state.forNode(edge)))
speculate(UnexpectedResizableArrayBufferView, jsValueValue(base), m_node, m_out.testNonZero32(m_out.load8ZeroExt32(base, m_heaps.JSArrayBufferView_mode), m_out.constInt32(isResizableOrGrowableSharedMode)));
#if USE(LARGE_TYPED_ARRAYS)
return m_out.load64NonNegative(base, m_heaps.JSArrayBufferView_length);
#else
Expand Down Expand Up @@ -5740,7 +5742,7 @@ IGNORE_CLANG_WARNINGS_END
default: {
DFG::ArrayMode arrayMode = m_node->arrayMode();
if (arrayMode.isSomeTypedArrayView()) {
LValue length = typedArrayLength(lowCell(m_node->child1()), arrayMode.mayBeResizableOrGrowableSharedTypedArray(), arrayMode.type() == Array::AnyTypedArray ? std::nullopt : std::optional { arrayMode.typedArrayType() });
LValue length = typedArrayLength(lowCell(m_node->child1()), arrayMode.mayBeResizableOrGrowableSharedTypedArray(), arrayMode.type() == Array::AnyTypedArray ? std::nullopt : std::optional { arrayMode.typedArrayType() }, m_node->child1());
#if USE(LARGE_TYPED_ARRAYS)
speculate(Overflow, noValue(), nullptr, m_out.above(length, m_out.constInt64(std::numeric_limits<int32_t>::max())));
setInt32(m_out.castToInt32(length));
Expand All @@ -5763,7 +5765,7 @@ IGNORE_CLANG_WARNINGS_BEGIN("missing-noreturn")
// The preprocessor chokes on RELEASE_ASSERT(USE(LARGE_TYPED_ARRAYS)), this is equivalent.
RELEASE_ASSERT(sizeof(size_t) == sizeof(uint64_t));
DFG::ArrayMode arrayMode = m_node->arrayMode();
setStrictInt52(typedArrayLength(lowCell(m_node->child1()), arrayMode.mayBeResizableOrGrowableSharedTypedArray(), arrayMode.type() == Array::AnyTypedArray ? std::nullopt : std::optional { arrayMode.typedArrayType() }));
setStrictInt52(typedArrayLength(lowCell(m_node->child1()), arrayMode.mayBeResizableOrGrowableSharedTypedArray(), arrayMode.type() == Array::AnyTypedArray ? std::nullopt : std::optional { arrayMode.typedArrayType() }, m_node->child1()));
}
IGNORE_CLANG_WARNINGS_END

Expand Down Expand Up @@ -17780,7 +17782,7 @@ IGNORE_CLANG_WARNINGS_END

DataViewData data = m_node->dataViewData();

LValue length = typedArrayLength(dataView, data.isResizable, TypeDataView);
LValue length = typedArrayLength(dataView, data.isResizable, TypeDataView, m_node->child1());

#if USE(LARGE_TYPED_ARRAYS)
speculate(OutOfBounds, noValue(), nullptr, m_out.lessThan(index, m_out.constInt32(0)));
Expand Down Expand Up @@ -17927,7 +17929,7 @@ IGNORE_CLANG_WARNINGS_END

DataViewData data = m_node->dataViewData();

LValue length = typedArrayLength(dataView, data.isResizable, TypeDataView);
LValue length = typedArrayLength(dataView, data.isResizable, TypeDataView, m_graph.varArgChild(m_node, 0));

#if USE(LARGE_TYPED_ARRAYS)
speculate(OutOfBounds, noValue(), nullptr, m_out.lessThan(index, m_out.constInt32(0)));
Expand Down Expand Up @@ -19435,25 +19437,6 @@ IGNORE_CLANG_WARNINGS_END

LValue masked = m_out.bitAnd(ptr, mask);
LValue result = m_out.add(masked, basePtr);
#if CPU(ARM64E)
result = m_out.select(
m_out.equal(ptr, m_out.constIntPtr(JSArrayBufferView::nullVectorPtr())),
ptr, result);
#endif

#if CPU(ARM64E)
if (kind == Gigacage::Primitive) {
PatchpointValue* merge = m_out.patchpoint(pointerType());
merge->append(result, B3::ValueRep(B3::ValueRep::SomeLateRegister));
merge->appendSomeRegister(ptr);
merge->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
jit.move(params[2].gpr(), params[0].gpr());
jit.insertBitField64(params[1].gpr(), CCallHelpers::TrustedImm32(0), CCallHelpers::TrustedImm32(64 - MacroAssembler::maxNumberOfAllowedPACBits), params[0].gpr());
});

result = merge;
}
#endif // CPU(ARM64E)

// Make sure that B3 doesn't try to do smart reassociation of these pointer bits.
// FIXME: In an ideal world, B3 would not do harmful reassociations, and if it did, it would be able
Expand Down

0 comments on commit eebb374

Please sign in to comment.