Title: [199277] trunk/Source/_javascript_Core
- Revision
- 199277
- Author
- sbar...@apple.com
- Date
- 2016-04-09 17:26:25 -0700 (Sat, 09 Apr 2016)
Log Message
Allocation sinking SSA Defs are allowed to have replacements
https://bugs.webkit.org/show_bug.cgi?id=156444
Reviewed by Filip Pizlo.
Consider the following program and the annotations that explain why
the SSA defs we create in allocation sinking can have replacements.
function foo(a1) {
let o1 = {x: 20, y: 50};
let o2 = {y: 40, o1: o1};
let o3 = {};
// We're Defing a new variable here, call it o3_field.
// o3_field is defing the value that is the result of
// a GetByOffset that gets eliminated through allocation sinking.
o3.field = o1.y;
dontCSE();
// This control flow is here to not allow the phase to consult
// its local SSA mapping (which properly handles replacements)
// for the value of o3_field.
if (a1) {
a1 = true;
} else {
a1 = false;
}
// Here, we ask for the reaching def of o3_field, and assert
// it doesn't have a replacement. It does have a replacement
// though. The original Def was the GetByOffset. We replaced
// that GetByOffset with the value of the o1_y variable.
let value = o3.field;
assert(value === 50);
}
* dfg/DFGObjectAllocationSinkingPhase.cpp:
* tests/stress/allocation-sinking-defs-may-have-replacements.js: Added.
(dontCSE):
(assert):
(foo):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (199276 => 199277)
--- trunk/Source/_javascript_Core/ChangeLog 2016-04-09 20:54:57 UTC (rev 199276)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-04-10 00:26:25 UTC (rev 199277)
@@ -1,3 +1,48 @@
+2016-04-09 Saam barati <sbar...@apple.com>
+
+ Allocation sinking SSA Defs are allowed to have replacements
+ https://bugs.webkit.org/show_bug.cgi?id=156444
+
+ Reviewed by Filip Pizlo.
+
+ Consider the following program and the annotations that explain why
+ the SSA defs we create in allocation sinking can have replacements.
+
+ function foo(a1) {
+ let o1 = {x: 20, y: 50};
+ let o2 = {y: 40, o1: o1};
+ let o3 = {};
+
+ // We're Defing a new variable here, call it o3_field.
+ // o3_field is defing the value that is the result of
+ // a GetByOffset that gets eliminated through allocation sinking.
+ o3.field = o1.y;
+
+ dontCSE();
+
+ // This control flow is here to not allow the phase to consult
+ // its local SSA mapping (which properly handles replacements)
+ // for the value of o3_field.
+ if (a1) {
+ a1 = true;
+ } else {
+ a1 = false;
+ }
+
+ // Here, we ask for the reaching def of o3_field, and assert
+ // it doesn't have a replacement. It does have a replacement
+ // though. The original Def was the GetByOffset. We replaced
+ // that GetByOffset with the value of the o1_y variable.
+ let value = o3.field;
+ assert(value === 50);
+ }
+
+ * dfg/DFGObjectAllocationSinkingPhase.cpp:
+ * tests/stress/allocation-sinking-defs-may-have-replacements.js: Added.
+ (dontCSE):
+ (assert):
+ (foo):
+
2016-04-09 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r199242.
Modified: trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp (199276 => 199277)
--- trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2016-04-09 20:54:57 UTC (rev 199276)
+++ trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2016-04-10 00:26:25 UTC (rev 199277)
@@ -1862,7 +1862,8 @@
ASSERT(def->value());
Node* result = def->value();
-
+ if (result->replacement())
+ result = result->replacement();
ASSERT(!result->replacement());
m_localMapping.add(location, result);
Added: trunk/Source/_javascript_Core/tests/stress/allocation-sinking-defs-may-have-replacements.js (0 => 199277)
--- trunk/Source/_javascript_Core/tests/stress/allocation-sinking-defs-may-have-replacements.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/allocation-sinking-defs-may-have-replacements.js 2016-04-10 00:26:25 UTC (rev 199277)
@@ -0,0 +1,31 @@
+function dontCSE() { }
+noInline(dontCSE);
+
+function assert(b) {
+ if (!b)
+ throw new Error("Bad assertion");
+}
+noInline(assert);
+
+function foo(a1) {
+ let o1 = {x: 20, y: 50};
+ let o2 = {y: 40, o1: o1};
+ let o3 = {};
+
+ o3.field = o1.y;
+
+ dontCSE();
+
+ if (a1) {
+ a1 = true;
+ } else {
+ a1 = false;
+ }
+
+ let value = o3.field;
+ assert(value === 50);
+}
+noInline(foo);
+
+for (let i = 0; i < 100000; i++)
+ foo(i);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
