Title: [250225] releases/WebKitGTK/webkit-2.26
- Revision
- 250225
- Author
- carlo...@webkit.org
- Date
- 2019-09-23 03:14:45 -0700 (Mon, 23 Sep 2019)
Log Message
Merge r250058 - Phantom insertion phase may disagree with arguments forwarding about live ranges
https://bugs.webkit.org/show_bug.cgi?id=200715
<rdar://problem/54301717>
Reviewed by Yusuke Suzuki.
JSTests:
* stress/phantom-insertion-live-range-should-agree-with-arguments-forwarding.js: Added.
(main.v23):
(main.try.v43):
(main.):
(main):
Source/_javascript_Core:
The issue is that Phantom insertion phase was disagreeing about live ranges
from the arguments forwarding phase. The effect is that Phantom insertion
would insert a Phantom creating a longer live range than what arguments
forwarding was analyzing. Arguments forwarding will look for the last DFG
use or the last bytecode use of a variable it wants to eliminate. It then
does an interference analysis to ensure that nothing clobbers other variables
it needs to recover the sunken allocation during OSR exit.
Phantom insertion works by ordering the program into OSR exit epochs. If a value was used
in the current epoch, there is no need to insert a phantom for it. We
determine where we might need a Phantom by looking at bytecode kills. In this
analysis, we have a mapping from bytecode local to DFG node. However, we
sometimes forgot to remove the entry when a local is killed. So, if the first
kill of a variable is in the same OSR exit epoch, we won't insert a Phantom by design.
However, if the variable gets killed again, we might errantly insert a Phantom
for the prior variable which should've already been killed. The solution is to
clear the entry in our mapping when a variable is killed.
The program in question was like this:
1: DirectArguments
...
2: MovHint(@1, loc1) // arguments forwarding treats this as the final kill for @1
...
clobber things needed for recovery
...
Arguments elimination would transform the program since between @1 and
@2, nothing clobbers values needed for exit and nothing escapes @1. The
program becomes:
1: PhantomDirectArguments
...
2: MovHint(@1, loc1) // arguments forwarding treats this as the final kill for @1
...
clobber things needed for recovery of @1
...
Phantom insertion would then transform the program into:
1: PhantomDirectArguments
...
2: MovHint(@1, loc1) // arguments forwarding treats this as the final kill for @1
...
clobber things needed for recovery of @1
...
3: Phantom(@1)
...
This is wrong because Phantom insertion and arguments forwarding must agree on live
ranges, otherwise the interference analysis performed by arguments forwarding will
not correctly analyze up until where the value might be recovered.
* dfg/DFGPhantomInsertionPhase.cpp:
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.26/JSTests/ChangeLog (250224 => 250225)
--- releases/WebKitGTK/webkit-2.26/JSTests/ChangeLog 2019-09-23 10:14:41 UTC (rev 250224)
+++ releases/WebKitGTK/webkit-2.26/JSTests/ChangeLog 2019-09-23 10:14:45 UTC (rev 250225)
@@ -1,3 +1,17 @@
+2019-09-18 Saam Barati <sbar...@apple.com>
+
+ Phantom insertion phase may disagree with arguments forwarding about live ranges
+ https://bugs.webkit.org/show_bug.cgi?id=200715
+ <rdar://problem/54301717>
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/phantom-insertion-live-range-should-agree-with-arguments-forwarding.js: Added.
+ (main.v23):
+ (main.try.v43):
+ (main.):
+ (main):
+
2019-09-16 Michael Saboff <msab...@apple.com>
[JSC] Perform check again when we found non-BMP characters
Added: releases/WebKitGTK/webkit-2.26/JSTests/stress/phantom-insertion-live-range-should-agree-with-arguments-forwarding.js (0 => 250225)
--- releases/WebKitGTK/webkit-2.26/JSTests/stress/phantom-insertion-live-range-should-agree-with-arguments-forwarding.js (rev 0)
+++ releases/WebKitGTK/webkit-2.26/JSTests/stress/phantom-insertion-live-range-should-agree-with-arguments-forwarding.js 2019-09-23 10:14:45 UTC (rev 250225)
@@ -0,0 +1,43 @@
+function main() {
+ const v2 = [1337,1337];
+ const v3 = [1337,v2,v2,0];
+ Object.__proto__ = v3;
+ for (let v10 = 0; v10 < 1000; v10++) {
+ function v11(v12,v13) {
+ var ____repro____;
+ const v15 = v10 + 127;
+ const v16 = String();
+ const v17 = String.fromCharCode(v10,v10,v15);
+ const v19 = Object.shift();
+ function v23() {
+ let v28 = arguments;
+ }
+ const v29 = Object();
+ const v30 = v23({},129);
+ const v31 = [-903931.176976766,v17,,,-903931.176976766];
+ const v32 = v31.join("");
+
+ try {
+ const v34 = Function(v32);
+ const v35 = v34();
+ for (let v39 = 0; v39 < 127; v39++) {
+ const v41 = isFinite();
+ let v42 = isFinite;
+ function v43(v44,v45,v46) {
+ }
+ const v47 = v41[4];
+ const v48 = v47[64];
+ const v49 = v35();
+ const v50 = v43();
+ const v51 = v34();
+ }
+ } catch(v52) {
+ }
+
+ }
+ const v53 = v11();
+ }
+}
+noDFG(main);
+noFTL(main);
+main();
Modified: releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/ChangeLog (250224 => 250225)
--- releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/ChangeLog 2019-09-23 10:14:41 UTC (rev 250224)
+++ releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/ChangeLog 2019-09-23 10:14:45 UTC (rev 250225)
@@ -1,3 +1,67 @@
+2019-09-18 Saam Barati <sbar...@apple.com>
+
+ Phantom insertion phase may disagree with arguments forwarding about live ranges
+ https://bugs.webkit.org/show_bug.cgi?id=200715
+ <rdar://problem/54301717>
+
+ Reviewed by Yusuke Suzuki.
+
+ The issue is that Phantom insertion phase was disagreeing about live ranges
+ from the arguments forwarding phase. The effect is that Phantom insertion
+ would insert a Phantom creating a longer live range than what arguments
+ forwarding was analyzing. Arguments forwarding will look for the last DFG
+ use or the last bytecode use of a variable it wants to eliminate. It then
+ does an interference analysis to ensure that nothing clobbers other variables
+ it needs to recover the sunken allocation during OSR exit.
+
+ Phantom insertion works by ordering the program into OSR exit epochs. If a value was used
+ in the current epoch, there is no need to insert a phantom for it. We
+ determine where we might need a Phantom by looking at bytecode kills. In this
+ analysis, we have a mapping from bytecode local to DFG node. However, we
+ sometimes forgot to remove the entry when a local is killed. So, if the first
+ kill of a variable is in the same OSR exit epoch, we won't insert a Phantom by design.
+ However, if the variable gets killed again, we might errantly insert a Phantom
+ for the prior variable which should've already been killed. The solution is to
+ clear the entry in our mapping when a variable is killed.
+
+ The program in question was like this:
+
+ 1: DirectArguments
+ ...
+ 2: MovHint(@1, loc1) // arguments forwarding treats this as the final kill for @1
+ ...
+ clobber things needed for recovery
+ ...
+
+ Arguments elimination would transform the program since between @1 and
+ @2, nothing clobbers values needed for exit and nothing escapes @1. The
+ program becomes:
+
+ 1: PhantomDirectArguments
+ ...
+ 2: MovHint(@1, loc1) // arguments forwarding treats this as the final kill for @1
+ ...
+ clobber things needed for recovery of @1
+ ...
+
+
+ Phantom insertion would then transform the program into:
+
+ 1: PhantomDirectArguments
+ ...
+ 2: MovHint(@1, loc1) // arguments forwarding treats this as the final kill for @1
+ ...
+ clobber things needed for recovery of @1
+ ...
+ 3: Phantom(@1)
+ ...
+
+ This is wrong because Phantom insertion and arguments forwarding must agree on live
+ ranges, otherwise the interference analysis performed by arguments forwarding will
+ not correctly analyze up until where the value might be recovered.
+
+ * dfg/DFGPhantomInsertionPhase.cpp:
+
2019-09-16 Michael Saboff <msab...@apple.com>
[JSC] Perform check again when we found non-BMP characters
Modified: releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGPhantomInsertionPhase.cpp (250224 => 250225)
--- releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGPhantomInsertionPhase.cpp 2019-09-23 10:14:41 UTC (rev 250224)
+++ releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGPhantomInsertionPhase.cpp 2019-09-23 10:14:45 UTC (rev 250225)
@@ -152,6 +152,8 @@
Node* killedNode = m_values.operand(reg);
if (!killedNode)
return;
+
+ m_values.operand(reg) = nullptr;
// We only need to insert a Phantom if the node hasn't been used since the last
// exit, and was born before the last exit.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
