ObjectAllocationSinkingPhase::insertOSRHintsForUpdate() fails to emit updated hints in some cases
https://bugs.webkit.org/show_bug.cgi?id=161492
Reviewed by Mark Lam.
JSTests:
This bug affected function->activation references but not object->object field references,
because object->object field references are !neededForMaterialization(). So, the object
test always passed but the activation/function test used to always fail. It passes now.
* stress/materialize-activation-referenced-from-phantom-function.js: Added.
(bar):
(inc):
(dec):
(foo):
(test):
* stress/materialize-object-referenced-from-phantom-object.js: Added.
(bar):
(foo):
(test):
Source/JavaScriptCore:
If you materialize a sunken object that is referenced from another sunken object, then you
have to emit a PutHint to tell OSR that the latter object now refers to a materialized
object rather than to the old sunken one.
The ObjectAllocationSinkingPhase totally knows how to do this, but for some reason it only
did it when the PromotedLocationDescriptor for the field used for referring to the other
object is !neededForMaterialization(), i.e. it's a NamedPropertyPLoc or a ClosureVarPLoc.
I can sort of imagine why we thought that would be right - neededForMaterialization() means
it's a special meta-data field initialized on construction. But just because it's immutable
and special doesn't mean that materialization can't change its physical representation.
Removing the requirement that it's !neededForMaterialization() fixes the test and doesn't
regress anything.
* dfg/DFGObjectAllocationSinkingPhase.cpp:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@205304 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp b/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp
index 4d34730..81c3061 100644
--- a/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp
@@ -1242,7 +1242,7 @@
for (const auto& field : entry.value.fields()) {
ASSERT(m_sinkCandidates.contains(entry.key) || !escapees.contains(field.value));
- if (escapees.contains(field.value) && !field.key.neededForMaterialization())
+ if (escapees.contains(field.value))
hints.append(PromotedHeapLocation(entry.key, field.key));
}
}
@@ -1692,6 +1692,8 @@
m_localMapping.set(location, m_bottom);
if (m_sinkCandidates.contains(node)) {
+ if (verbose)
+ dataLog("For sink candidate ", node, " found location ", location, "\n");
m_insertionSet.insert(
nodeIndex + 1,
location.createHint(
@@ -1758,6 +1760,8 @@
doLower = true;
+ if (verbose)
+ dataLog("Creating hint with value ", nodeValue, " before ", node, "\n");
m_insertionSet.insert(
nodeIndex + 1,
location.createHint(
@@ -1893,6 +1897,13 @@
void insertOSRHintsForUpdate(unsigned nodeIndex, NodeOrigin origin, bool& canExit, AvailabilityMap& availability, Node* escapee, Node* materialization)
{
+ if (verbose) {
+ dataLog("Inserting OSR hints at ", origin, ":\n");
+ dataLog(" Escapee: ", escapee, "\n");
+ dataLog(" Materialization: ", materialization, "\n");
+ dataLog(" Availability: ", availability, "\n");
+ }
+
// We need to follow() the value in the heap.
// Consider the following graph:
//
