Skip to content

Commit 7d6c85e

Browse files
committed
[JSC] ASSERTION FAILED: child(0)->type() == phi->type()
rdar://147364146 https://bugs.webkit.org/show_bug.cgi?id=290009 Reviewed by Yusuke Suzuki. Array allocation sinking introduced cases where Upsilon edges may come from nodes with result types like double or int52, instead of the previously assumed JS result type. This breaks assumptions made during Phi construction, which always expected NodeResultJS. This patch adds `fixUpsilonEdge()`, a pass that ensures Upsilon inputs match the expected Phi result type by inserting appropriate ValueRep nodes when needed. * JSTests/stress/array-allocation-sink-upsilon-with-double-value.js: Added. (test): * Source/JavaScriptCore/dfg/DFGFlushFormat.h: Canonical link: https://commits.webkit.org/292572@main
1 parent b9ce71c commit 7d6c85e

2 files changed

Lines changed: 50 additions & 0 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
function test(x) {
3+
const a = new Array(4);
4+
let v = 50;
5+
for (let i = 0; i < 100; v++) {
6+
a[i] = Math.random();
7+
if (v % 9 == 0)
8+
return;
9+
}
10+
}
11+
12+
for (let i = 0; i < testLoopCount; ++i) {
13+
test(i);
14+
}

Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,7 @@ class ObjectAllocationSinkingPhase : public Phase {
862862

863863
promoteLocalHeap();
864864
removeICStatusFilters();
865+
fixUpsilonEdge();
865866

866867
if (UNLIKELY(Options::validateGraphAtEachPhase()))
867868
DFG::validate(m_graph, DumpGraph, graphBeforeSinking);
@@ -2819,6 +2820,41 @@ class ObjectAllocationSinkingPhase : public Phase {
28192820
}
28202821
}
28212822

2823+
// The added phis have NodeResultJS because their corresponding Upsilon edges, when coming from nodes in
2824+
// NamedPropertyPLoc, always have use kinds associated with NodeResultJS. However, this assumption breaks
2825+
// with the introduction of array allocation sinking, since nodes in ArrayIndexedPropertyPLoc may have
2826+
// use kinds that produce double results.
2827+
void fixUpsilonEdge()
2828+
{
2829+
for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
2830+
for (unsigned indexInBlock = 0; indexInBlock < block->size(); ++indexInBlock) {
2831+
Node* node = block->at(indexInBlock);
2832+
switch (node->op()) {
2833+
case Upsilon: {
2834+
Edge& edge = node->child1();
2835+
if (node->phi()->hasJSResult()) {
2836+
Node* result = nullptr;
2837+
if (edge->hasDoubleResult())
2838+
result = m_insertionSet.insertNode(indexInBlock, SpecBytecodeDouble, ValueRep, node->origin, Edge(edge.node(), DoubleRepUse));
2839+
else if (edge->hasInt52Result())
2840+
result = m_insertionSet.insertNode(indexInBlock, SpecInt32Only | SpecAnyIntAsDouble, ValueRep, node->origin, Edge(edge.node(), Int52RepUse));
2841+
2842+
if (result) {
2843+
edge.setNode(result);
2844+
edge.setUseKind(UntypedUse);
2845+
}
2846+
}
2847+
break;
2848+
}
2849+
default:
2850+
break;
2851+
}
2852+
}
2853+
2854+
m_insertionSet.execute(block);
2855+
}
2856+
}
2857+
28222858
// This is a great way of asking value->isStillValid() without having to worry about getting
28232859
// different answers. It turns out that this analysis works OK regardless of what this
28242860
// returns but breaks badly if this changes its mind for any particular InferredValue. This

0 commit comments

Comments
 (0)