Title: [254866] trunk
- Revision
- 254866
- Author
- [email protected]
- Date
- 2020-01-21 11:36:05 -0800 (Tue, 21 Jan 2020)
Log Message
JSTests:
Object allocation sinking is missing PutHint for allocations unreachable in the graph
https://bugs.webkit.org/show_bug.cgi?id=203799
<rdar://problem/56852162>
Reviewed by Saam Barati.
* stress/allocation-sinking-puthint-control-flow-2.js: Added.
(f.handler.construct):
(f):
Source/_javascript_Core:
Object allocation sinking is missing PutHint for sunken allocations
https://bugs.webkit.org/show_bug.cgi?id=203799
<rdar://problem/56852162>
Reviewed by Saam Barati.
Consider the following graph:
Block #0:
1: PhantomCreateActivation()
2: PhantomNewFunction()
PutHint(@2, @1, FunctionActivationPLoc)
Branch(#1, #2)
Block #1:
3: MaterializeCreateActivation()
PutHint(@2, @3, FunctionActivationPLoc)
Upsilon(@3, ^5)
Jump(#3)
Block #2:
4: MaterializeCreateActivation()
PutHint(@2, @4, FunctionActivationPLoc)
Upsilon(@4, ^5)
Jump(#3)
Block #3:
5: Phi()
ExitOK()
On Block #3, we need to emit a PutHint after the Phi, since we might exit after it. However,
object allocation sinking skipped this Phi because it was checking whether the base of the
location that caused us to create this Phi (@2) was live, but it's dead in the graph (there
are no pointers to it). The issue is that, even though there are no pointers to the base, the
location `PromotedHeapLocation(@2, FunctionActivationPLoc)` is still live, so we should PutHint
to it. We fix it by checking for liveness of the location rather than its base.
* dfg/DFGObjectAllocationSinkingPhase.cpp:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (254865 => 254866)
--- trunk/JSTests/ChangeLog 2020-01-21 19:30:43 UTC (rev 254865)
+++ trunk/JSTests/ChangeLog 2020-01-21 19:36:05 UTC (rev 254866)
@@ -1,3 +1,15 @@
+2020-01-21 Tadeu Zagallo <[email protected]>
+
+ Object allocation sinking is missing PutHint for allocations unreachable in the graph
+ https://bugs.webkit.org/show_bug.cgi?id=203799
+ <rdar://problem/56852162>
+
+ Reviewed by Saam Barati.
+
+ * stress/allocation-sinking-puthint-control-flow-2.js: Added.
+ (f.handler.construct):
+ (f):
+
2020-01-20 Gus Caplan <[email protected]>
Remove own toString from NativeError prototype
Added: trunk/JSTests/stress/allocation-sinking-puthint-control-flow-2.js (0 => 254866)
--- trunk/JSTests/stress/allocation-sinking-puthint-control-flow-2.js (rev 0)
+++ trunk/JSTests/stress/allocation-sinking-puthint-control-flow-2.js 2020-01-21 19:36:05 UTC (rev 254866)
@@ -0,0 +1,16 @@
+//@ runDefault("--useConcurrentJIT=0", "--jitPolicyScale=0")
+
+function f() {
+ var x = {};
+ x = 0;
+ var handler = {
+ construct: function () {
+ x;
+ }
+ };
+ for (let i = 0; i < 1; i++)
+ (function () { i });
+ new Proxy(function() { }, handler);
+}
+f();
+f();
Modified: trunk/Source/_javascript_Core/ChangeLog (254865 => 254866)
--- trunk/Source/_javascript_Core/ChangeLog 2020-01-21 19:30:43 UTC (rev 254865)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-01-21 19:36:05 UTC (rev 254866)
@@ -1,3 +1,44 @@
+2020-01-21 Tadeu Zagallo <[email protected]>
+
+ Object allocation sinking is missing PutHint for sunken allocations
+ https://bugs.webkit.org/show_bug.cgi?id=203799
+ <rdar://problem/56852162>
+
+ Reviewed by Saam Barati.
+
+ Consider the following graph:
+
+ Block #0:
+ 1: PhantomCreateActivation()
+ 2: PhantomNewFunction()
+ PutHint(@2, @1, FunctionActivationPLoc)
+ Branch(#1, #2)
+
+ Block #1:
+ 3: MaterializeCreateActivation()
+ PutHint(@2, @3, FunctionActivationPLoc)
+ Upsilon(@3, ^5)
+ Jump(#3)
+
+ Block #2:
+ 4: MaterializeCreateActivation()
+ PutHint(@2, @4, FunctionActivationPLoc)
+ Upsilon(@4, ^5)
+ Jump(#3)
+
+ Block #3:
+ 5: Phi()
+ ExitOK()
+
+ On Block #3, we need to emit a PutHint after the Phi, since we might exit after it. However,
+ object allocation sinking skipped this Phi because it was checking whether the base of the
+ location that caused us to create this Phi (@2) was live, but it's dead in the graph (there
+ are no pointers to it). The issue is that, even though there are no pointers to the base, the
+ location `PromotedHeapLocation(@2, FunctionActivationPLoc)` is still live, so we should PutHint
+ to it. We fix it by checking for liveness of the location rather than its base.
+
+ * dfg/DFGObjectAllocationSinkingPhase.cpp:
+
2020-01-21 Mark Lam <[email protected]>
Rename JSPromiseFields abstract heap to JSInternalFields.
Modified: trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp (254865 => 254866)
--- trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2020-01-21 19:30:43 UTC (rev 254865)
+++ trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2020-01-21 19:36:05 UTC (rev 254866)
@@ -410,7 +410,6 @@
{
const Allocation& base = m_allocations.find(location.base())->value;
auto iter = base.fields().find(location.descriptor());
-
if (iter == base.fields().end())
return nullptr;
@@ -438,6 +437,12 @@
return &getAllocation(identifier);
}
+ bool isUnescapedAllocation(Node* identifier) const
+ {
+ auto iter = m_allocations.find(identifier);
+ return iter != m_allocations.end() && !iter->value.isEscapedAllocation();
+ }
+
// This allows us to store the escapees only when necessary. If
// set, the current escapees can be retrieved at any time using
// takeEscapees(), which will clear the cached set of escapees;
@@ -1946,7 +1951,7 @@
availabilityCalculator.m_availability, identifier, phiDef->value());
for (PromotedHeapLocation location : hintsForPhi[variable->index()]) {
- if (m_heap.onlyLocalAllocation(location.base())) {
+ if (m_heap.isUnescapedAllocation(location.base())) {
m_insertionSet.insert(0,
location.createHint(m_graph, block->at(0)->origin.withInvalidExit(), phiDef->value()));
m_localMapping.set(location, phiDef->value());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
