Title: [236421] trunk
- Revision
- 236421
- Author
- [email protected]
- Date
- 2018-09-24 13:12:26 -0700 (Mon, 24 Sep 2018)
Log Message
ArgumentsEliminationPhase should snip basic blocks after proven OSR exits
https://bugs.webkit.org/show_bug.cgi?id=189682
<rdar://problem/43557315>
Reviewed by Mark Lam.
JSTests:
* stress/arguments-elimination-will-generate-edge-without-result.js: Added.
(foo):
Source/_javascript_Core:
Otherwise, if we have code like this:
```
a: Arguments
b: GetButterfly(@a)
c: ForceExit
d: GetArrayLength(@a, @b)
```
it will get transformed into this invalid DFG IR:
```
a: PhantomArguments
b: Check(@a)
c: ForceExit
d: GetArrayLength(@a, @b)
```
And we will fail DFG validation since @b does not have a result.
The fix is to just remove all nodes after the ForceExit and plant an
Unreachable after it. So the above code program will now turn into this:
```
a: PhantomArguments
b: Check(@a)
c: ForceExit
e: Unreachable
```
* dfg/DFGArgumentsEliminationPhase.cpp:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (236420 => 236421)
--- trunk/JSTests/ChangeLog 2018-09-24 19:44:08 UTC (rev 236420)
+++ trunk/JSTests/ChangeLog 2018-09-24 20:12:26 UTC (rev 236421)
@@ -1,3 +1,14 @@
+2018-09-24 Saam barati <[email protected]>
+
+ ArgumentsEliminationPhase should snip basic blocks after proven OSR exits
+ https://bugs.webkit.org/show_bug.cgi?id=189682
+ <rdar://problem/43557315>
+
+ Reviewed by Mark Lam.
+
+ * stress/arguments-elimination-will-generate-edge-without-result.js: Added.
+ (foo):
+
2018-09-22 Saam barati <[email protected]>
The sampling should not use Strong<CodeBlock> in its machineLocation field
Added: trunk/JSTests/stress/arguments-elimination-will-generate-edge-without-result.js (0 => 236421)
--- trunk/JSTests/stress/arguments-elimination-will-generate-edge-without-result.js (rev 0)
+++ trunk/JSTests/stress/arguments-elimination-will-generate-edge-without-result.js 2018-09-24 20:12:26 UTC (rev 236421)
@@ -0,0 +1,9 @@
+//@ runDefault("--validateGraphAtEachPhase=true", "--jitPolicyScale=0", "--useConcurrentJIT=0")
+
+'use strict';
+function foo() {
+ return arguments[1][0] === arguments[0];
+}
+for (let i = 0; i < 100000; ++i) {
+ foo(0, 0);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (236420 => 236421)
--- trunk/Source/_javascript_Core/ChangeLog 2018-09-24 19:44:08 UTC (rev 236420)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-09-24 20:12:26 UTC (rev 236421)
@@ -1,3 +1,39 @@
+2018-09-24 Saam barati <[email protected]>
+
+ ArgumentsEliminationPhase should snip basic blocks after proven OSR exits
+ https://bugs.webkit.org/show_bug.cgi?id=189682
+ <rdar://problem/43557315>
+
+ Reviewed by Mark Lam.
+
+ Otherwise, if we have code like this:
+ ```
+ a: Arguments
+ b: GetButterfly(@a)
+ c: ForceExit
+ d: GetArrayLength(@a, @b)
+ ```
+ it will get transformed into this invalid DFG IR:
+ ```
+ a: PhantomArguments
+ b: Check(@a)
+ c: ForceExit
+ d: GetArrayLength(@a, @b)
+ ```
+
+ And we will fail DFG validation since @b does not have a result.
+
+ The fix is to just remove all nodes after the ForceExit and plant an
+ Unreachable after it. So the above code program will now turn into this:
+ ```
+ a: PhantomArguments
+ b: Check(@a)
+ c: ForceExit
+ e: Unreachable
+ ```
+
+ * dfg/DFGArgumentsEliminationPhase.cpp:
+
2018-09-22 Saam barati <[email protected]>
The sampling should not use Strong<CodeBlock> in its machineLocation field
Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (236420 => 236421)
--- trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2018-09-24 19:44:08 UTC (rev 236420)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2018-09-24 20:12:26 UTC (rev 236421)
@@ -624,9 +624,12 @@
void transform()
{
+ bool modifiedCFG = false;
+
InsertionSet insertionSet(m_graph);
-
+
for (BasicBlock* block : m_graph.blocksInPreOrder()) {
+ Node* pseudoTerminal = nullptr;
for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
Node* node = block->at(nodeIndex);
@@ -1210,12 +1213,33 @@
default:
break;
}
- if (node->isPseudoTerminal())
+
+ if (node->isPseudoTerminal()) {
+ pseudoTerminal = node;
break;
+ }
}
-
+
insertionSet.execute(block);
+
+ if (pseudoTerminal) {
+ for (unsigned i = 0; i < block->size(); ++i) {
+ Node* node = block->at(i);
+ if (node != pseudoTerminal)
+ continue;
+ block->resize(i + 1);
+ block->append(m_graph.addNode(SpecNone, Unreachable, node->origin));
+ modifiedCFG = true;
+ break;
+ }
+ }
}
+
+ if (modifiedCFG) {
+ m_graph.invalidateCFG();
+ m_graph.resetReachability();
+ m_graph.killUnreachableBlocks();
+ }
}
HashSet<Node*> m_candidates;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
