Title: [245341] trunk
- Revision
- 245341
- Author
- [email protected]
- Date
- 2019-05-15 13:30:16 -0700 (Wed, 15 May 2019)
Log Message
Bound liveness of SetArgumentMaybe nodes when maximal flush insertion phase is enabled
https://bugs.webkit.org/show_bug.cgi?id=197855
<rdar://problem/50236506>
Reviewed by Michael Saboff.
JSTests:
* stress/set-argument-maybe-maximal-flush-should-not-extend-liveness-2.js: Added.
(f0):
(bar):
(foo):
* stress/set-argument-maybe-maximal-flush-should-not-extend-liveness.js: Added.
(f1):
(f2):
(foo):
Source/_javascript_Core:
Maximal flush insertion phase assumes it can extend the live range of
variables. However, this is not true with SetArgumentMaybe nodes, because
they are not guaranteed to demarcate the birth of a variable in the way
that SetArgumentDefinitely does. This caused things to break in SSA conversion
when we wanted to use the result of a SetArgumentMaybe node. To obviate this,
when we're done inlining something with SetArgumentMaybes, we SetLocal(undefined)
to the same set of locals. This caps the live range of the SetArgumentMaybe
and makes it so that extending the live range of the SetLocal is valid.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleVarargsInlining):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (245340 => 245341)
--- trunk/JSTests/ChangeLog 2019-05-15 19:56:40 UTC (rev 245340)
+++ trunk/JSTests/ChangeLog 2019-05-15 20:30:16 UTC (rev 245341)
@@ -1,3 +1,20 @@
+2019-05-15 Saam Barati <[email protected]>
+
+ Bound liveness of SetArgumentMaybe nodes when maximal flush insertion phase is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=197855
+ <rdar://problem/50236506>
+
+ Reviewed by Michael Saboff.
+
+ * stress/set-argument-maybe-maximal-flush-should-not-extend-liveness-2.js: Added.
+ (f0):
+ (bar):
+ (foo):
+ * stress/set-argument-maybe-maximal-flush-should-not-extend-liveness.js: Added.
+ (f1):
+ (f2):
+ (foo):
+
2019-05-14 Keith Miller <[email protected]>
Fix issue with byteOffset on ARM64E
Added: trunk/JSTests/stress/set-argument-maybe-maximal-flush-should-not-extend-liveness-2.js (0 => 245341)
--- trunk/JSTests/stress/set-argument-maybe-maximal-flush-should-not-extend-liveness-2.js (rev 0)
+++ trunk/JSTests/stress/set-argument-maybe-maximal-flush-should-not-extend-liveness-2.js 2019-05-15 20:30:16 UTC (rev 245341)
@@ -0,0 +1,20 @@
+//@ runDefault("--useMaximalFlushInsertionPhase=1", "--jitPolicyScale=0", "--useConcurrentJIT=0")
+
+function f0() {
+}
+
+function bar() {
+ f0(...arguments);
+}
+
+const a = new Uint8Array(1);
+
+function foo() {
+ bar(0, 0);
+ a.find(()=>{});
+}
+
+
+for (let i=0; i<3; i++) {
+ foo();
+}
Added: trunk/JSTests/stress/set-argument-maybe-maximal-flush-should-not-extend-liveness.js (0 => 245341)
--- trunk/JSTests/stress/set-argument-maybe-maximal-flush-should-not-extend-liveness.js (rev 0)
+++ trunk/JSTests/stress/set-argument-maybe-maximal-flush-should-not-extend-liveness.js 2019-05-15 20:30:16 UTC (rev 245341)
@@ -0,0 +1,20 @@
+//@ runDefault("--useMaximalFlushInsertionPhase=1", "--validateGraphAtEachPhase=1")
+
+function f1() {
+}
+
+function f2() {
+}
+
+const a = [0];
+
+function foo() {
+ f1(...a);
+ for (let i = 0; i < 2; i++) {
+ f2([] > 0);
+ }
+}
+
+for (var i = 0; i < 1000000; ++i) {
+ foo();
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (245340 => 245341)
--- trunk/Source/_javascript_Core/ChangeLog 2019-05-15 19:56:40 UTC (rev 245340)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-05-15 20:30:16 UTC (rev 245341)
@@ -1,3 +1,23 @@
+2019-05-15 Saam Barati <[email protected]>
+
+ Bound liveness of SetArgumentMaybe nodes when maximal flush insertion phase is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=197855
+ <rdar://problem/50236506>
+
+ Reviewed by Michael Saboff.
+
+ Maximal flush insertion phase assumes it can extend the live range of
+ variables. However, this is not true with SetArgumentMaybe nodes, because
+ they are not guaranteed to demarcate the birth of a variable in the way
+ that SetArgumentDefinitely does. This caused things to break in SSA conversion
+ when we wanted to use the result of a SetArgumentMaybe node. To obviate this,
+ when we're done inlining something with SetArgumentMaybes, we SetLocal(undefined)
+ to the same set of locals. This caps the live range of the SetArgumentMaybe
+ and makes it so that extending the live range of the SetLocal is valid.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleVarargsInlining):
+
2019-05-14 Keith Miller <[email protected]>
Fix issue with byteOffset on ARM64E
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (245340 => 245341)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2019-05-15 19:56:40 UTC (rev 245340)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2019-05-15 20:30:16 UTC (rev 245341)
@@ -1863,6 +1863,8 @@
registerOffset -= maxNumArguments; // includes "this"
registerOffset -= CallFrame::headerSizeInRegisters;
registerOffset = -WTF::roundUpToMultipleOf(stackAlignmentRegisters(), -registerOffset);
+
+ Vector<VirtualRegister> setArgumentMaybes;
auto insertChecks = [&] (CodeBlock* codeBlock) {
emitFunctionChecks(callVariant, callTargetNode, thisArgument);
@@ -1928,6 +1930,8 @@
}
Node* setArgument = addToGraph(numSetArguments >= mandatoryMinimum ? SetArgumentMaybe : SetArgumentDefinitely, OpInfo(variable));
+ if (numSetArguments >= mandatoryMinimum && Options::useMaximalFlushInsertionPhase())
+ setArgumentMaybes.append(variable->local());
m_currentBlock->variablesAtTail.setOperand(variable->local(), setArgument);
++numSetArguments;
}
@@ -1942,6 +1946,9 @@
// calling LoadVarargs twice.
inlineCall(callTargetNode, result, callVariant, registerOffset, maxNumArguments, kind, nullptr, insertChecks);
+ for (VirtualRegister reg : setArgumentMaybes)
+ setDirect(reg, jsConstant(jsUndefined()), ImmediateNakedSet);
+
VERBOSE_LOG("Successful inlining (varargs, monomorphic).\nStack: ", currentCodeOrigin(), "\n");
return true;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
