Modified: releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog (248234 => 248235)
--- releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog 2019-08-04 03:23:17 UTC (rev 248234)
+++ releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog 2019-08-04 03:23:19 UTC (rev 248235)
@@ -1,5 +1,17 @@
2019-06-04 Tadeu Zagallo <[email protected]>
+ Argument elimination should check transitive dependents for interference
+ https://bugs.webkit.org/show_bug.cgi?id=198520
+ <rdar://problem/50863343>
+
+ Reviewed by Filip Pizlo.
+
+ * stress/argument-elimination-inline-rest-past-kill.js: Added.
+ (f2):
+ (f3):
+
+2019-06-04 Tadeu Zagallo <[email protected]>
+
Argument elimination should check for negative indices in GetByVal
https://bugs.webkit.org/show_bug.cgi?id=198302
<rdar://problem/51188095>
Added: releases/WebKitGTK/webkit-2.24/JSTests/stress/argument-elimination-inline-rest-past-kill.js (0 => 248235)
--- releases/WebKitGTK/webkit-2.24/JSTests/stress/argument-elimination-inline-rest-past-kill.js (rev 0)
+++ releases/WebKitGTK/webkit-2.24/JSTests/stress/argument-elimination-inline-rest-past-kill.js 2019-08-04 03:23:19 UTC (rev 248235)
@@ -0,0 +1,16 @@
+//@ requireOptions("--forceEagerCompilation=1")
+function f2(...a1) {
+ return a1;
+}
+
+function f3(...a2) {
+ let v1 = f2([]);
+ return f2(...a2, ...v1);
+}
+noInline(f3);
+
+for (let i = 0; i < 1e5; i++) {
+ var v3 = f3();
+ if (!Array.isArray(v3[v3.length - 1]))
+ throw new Error('Should be an array');
+}
Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog (248234 => 248235)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog 2019-08-04 03:23:17 UTC (rev 248234)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog 2019-08-04 03:23:19 UTC (rev 248235)
@@ -1,5 +1,33 @@
2019-06-04 Tadeu Zagallo <[email protected]>
+ Argument elimination should check transitive dependents for interference
+ https://bugs.webkit.org/show_bug.cgi?id=198520
+ <rdar://problem/50863343>
+
+ Reviewed by Filip Pizlo.
+
+ Consider the following program:
+
+ a: CreateRest
+ -->
+ b: CreateRest
+ <--
+ c: Spread(@a)
+ d: Spread(@b)
+ e: NewArrayWithSpread(@a, @b)
+ f: KillStack(locX)
+ g: LoadVarargs(@e)
+
+ Suppose @b reads locX, then we cannot transform @e to PhantomNewArraySpread, since that would
+ move the stack access from @b into @g, and that stack location is no longer valid at that point.
+
+ We fix that by computing a set of all inline call frames that any argument elimination candidate
+ depends on and checking each of them for interference in `eliminateCandidatesThatInterfere`.
+
+ * dfg/DFGArgumentsEliminationPhase.cpp:
+
+2019-06-04 Tadeu Zagallo <[email protected]>
+
Argument elimination should check for negative indices in GetByVal
https://bugs.webkit.org/show_bug.cgi?id=198302
<rdar://problem/51188095>
Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (248234 => 248235)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2019-08-04 03:23:17 UTC (rev 248234)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2019-08-04 03:23:19 UTC (rev 248235)
@@ -502,6 +502,33 @@
}
}
+ using InlineCallFrames = HashSet<InlineCallFrame*, WTF::DefaultHash<InlineCallFrame*>::Hash, WTF::NullableHashTraits<InlineCallFrame*>>;
+ using InlineCallFramesForCanditates = HashMap<Node*, InlineCallFrames>;
+ InlineCallFramesForCanditates inlineCallFramesForCandidate;
+ auto forEachDependentNode = recursableLambda([&](auto self, Node* node, const auto& functor) -> void {
+ functor(node);
+
+ if (node->op() == Spread) {
+ self(node->child1().node(), functor);
+ return;
+ }
+
+ if (node->op() == NewArrayWithSpread) {
+ BitVector* bitVector = node->bitVector();
+ for (unsigned i = node->numChildren(); i--; ) {
+ if (bitVector->get(i))
+ self(m_graph.varArgChild(node, i).node(), functor);
+ }
+ return;
+ }
+ });
+ for (Node* candidate : m_candidates) {
+ auto& set = inlineCallFramesForCandidate.add(candidate, InlineCallFrames()).iterator->value;
+ forEachDependentNode(candidate, [&](Node* dependent) {
+ set.add(dependent->origin.semantic.inlineCallFrame);
+ });
+ }
+
for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
// Stop if we've already removed all candidates.
if (m_candidates.isEmpty())
@@ -524,83 +551,85 @@
if (!m_candidates.contains(candidate))
return;
- // Check if this block has any clobbers that affect this candidate. This is a fairly
- // fast check.
- bool isClobberedByBlock = false;
- Operands<bool>& clobberedByThisBlock = clobberedByBlock[block];
-
- if (InlineCallFrame* inlineCallFrame = candidate->origin.semantic.inlineCallFrame) {
- if (inlineCallFrame->isVarargs()) {
- isClobberedByBlock |= clobberedByThisBlock.operand(
- inlineCallFrame->stackOffset + CallFrameSlot::argumentCount);
- }
+ for (InlineCallFrame* inlineCallFrame : inlineCallFramesForCandidate.get(candidate)) {
+ // Check if this block has any clobbers that affect this candidate. This is a fairly
+ // fast check.
+ bool isClobberedByBlock = false;
+ Operands<bool>& clobberedByThisBlock = clobberedByBlock[block];
- if (!isClobberedByBlock || inlineCallFrame->isClosureCall) {
- isClobberedByBlock |= clobberedByThisBlock.operand(
- inlineCallFrame->stackOffset + CallFrameSlot::callee);
- }
-
- if (!isClobberedByBlock) {
- for (unsigned i = 0; i < inlineCallFrame->argumentCountIncludingThis - 1; ++i) {
- VirtualRegister reg =
- VirtualRegister(inlineCallFrame->stackOffset) +
- CallFrame::argumentOffset(i);
- if (clobberedByThisBlock.operand(reg)) {
+ if (inlineCallFrame) {
+ if (inlineCallFrame->isVarargs()) {
+ isClobberedByBlock |= clobberedByThisBlock.operand(
+ inlineCallFrame->stackOffset + CallFrameSlot::argumentCount);
+ }
+
+ if (!isClobberedByBlock || inlineCallFrame->isClosureCall) {
+ isClobberedByBlock |= clobberedByThisBlock.operand(
+ inlineCallFrame->stackOffset + CallFrameSlot::callee);
+ }
+
+ if (!isClobberedByBlock) {
+ for (unsigned i = 0; i < inlineCallFrame->argumentCountIncludingThis - 1; ++i) {
+ VirtualRegister reg =
+ VirtualRegister(inlineCallFrame->stackOffset) +
+ CallFrame::argumentOffset(i);
+ if (clobberedByThisBlock.operand(reg)) {
+ isClobberedByBlock = true;
+ break;
+ }
+ }
+ }
+ } else {
+ // We don't include the ArgumentCount or Callee in this case because we can be
+ // damn sure that this won't be clobbered.
+ for (unsigned i = 1; i < static_cast<unsigned>(codeBlock()->numParameters()); ++i) {
+ if (clobberedByThisBlock.argument(i)) {
isClobberedByBlock = true;
break;
}
}
}
- } else {
- // We don't include the ArgumentCount or Callee in this case because we can be
- // damn sure that this won't be clobbered.
- for (unsigned i = 1; i < static_cast<unsigned>(codeBlock()->numParameters()); ++i) {
- if (clobberedByThisBlock.argument(i)) {
- isClobberedByBlock = true;
- break;
- }
- }
- }
-
- if (!isClobberedByBlock)
- return;
-
- // Check if we can immediately eliminate this candidate. If the block has a clobber
- // for this arguments allocation, and we'd have to examine every node in the block,
- // then we can just eliminate the candidate.
- if (nodeIndex == block->size() && candidate->owner != block) {
- if (DFGArgumentsEliminationPhaseInternal::verbose)
- dataLog("eliminating candidate: ", candidate, " because it is clobbered by: ", block->at(nodeIndex), "\n");
- transitivelyRemoveCandidate(candidate);
- return;
- }
-
- // This loop considers all nodes up to the nodeIndex, excluding the nodeIndex.
- while (nodeIndex--) {
- Node* node = block->at(nodeIndex);
- if (node == candidate)
- break;
- bool found = false;
- clobberize(
- m_graph, node, NoOpClobberize(),
- [&] (AbstractHeap heap) {
- if (heap.kind() == Stack && !heap.payload().isTop()) {
- if (argumentsInvolveStackSlot(candidate, VirtualRegister(heap.payload().value32())))
- found = true;
- return;
- }
- if (heap.overlaps(Stack))
- found = true;
- },
- NoOpClobberize());
+ if (!isClobberedByBlock)
+ continue;
- if (found) {
+ // Check if we can immediately eliminate this candidate. If the block has a clobber
+ // for this arguments allocation, and we'd have to examine every node in the block,
+ // then we can just eliminate the candidate.
+ if (nodeIndex == block->size() && candidate->owner != block) {
if (DFGArgumentsEliminationPhaseInternal::verbose)
- dataLog("eliminating candidate: ", candidate, " because it is clobbered by ", block->at(nodeIndex), "\n");
+ dataLog("eliminating candidate: ", candidate, " because it is clobbered by: ", block->at(nodeIndex), "\n");
transitivelyRemoveCandidate(candidate);
return;
}
+
+ // This loop considers all nodes up to the nodeIndex, excluding the nodeIndex.
+ while (nodeIndex--) {
+ Node* node = block->at(nodeIndex);
+ if (node == candidate)
+ break;
+
+ bool found = false;
+ clobberize(
+ m_graph, node, NoOpClobberize(),
+ [&] (AbstractHeap heap) {
+ if (heap.kind() == Stack && !heap.payload().isTop()) {
+ if (argumentsInvolveStackSlot(inlineCallFrame, VirtualRegister(heap.payload().value32())))
+ found = true;
+ return;
+ }
+ if (heap.overlaps(Stack))
+ found = true;
+ },
+ NoOpClobberize());
+
+ if (found) {
+ if (DFGArgumentsEliminationPhaseInternal::verbose)
+ dataLog("eliminating candidate: ", candidate, " because it is clobbered by ", block->at(nodeIndex), "\n");
+ transitivelyRemoveCandidate(candidate);
+ return;
+ }
+ }
}
});
}
