Title: [183719] trunk/Source/_javascript_Core
- Revision
- 183719
- Author
- fpi...@apple.com
- Date
- 2015-05-02 11:36:24 -0700 (Sat, 02 May 2015)
Log Message
Unreviewed, add a FIXME referencing https://bugs.webkit.org/show_bug.cgi?id=144527.
* dfg/DFGLICMPhase.cpp:
(JSC::DFG::LICMPhase::attemptHoist):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (183718 => 183719)
--- trunk/Source/_javascript_Core/ChangeLog 2015-05-02 17:33:36 UTC (rev 183718)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-05-02 18:36:24 UTC (rev 183719)
@@ -1,5 +1,12 @@
2015-05-02 Filip Pizlo <fpi...@apple.com>
+ Unreviewed, add a FIXME referencing https://bugs.webkit.org/show_bug.cgi?id=144527.
+
+ * dfg/DFGLICMPhase.cpp:
+ (JSC::DFG::LICMPhase::attemptHoist):
+
+2015-05-02 Filip Pizlo <fpi...@apple.com>
+
Unreviewed, add FIXMEs referencing https://bugs.webkit.org/show_bug.cgi?id=144524 and
https://bugs.webkit.org/show_bug.cgi?id=144525.
Modified: trunk/Source/_javascript_Core/dfg/DFGLICMPhase.cpp (183718 => 183719)
--- trunk/Source/_javascript_Core/dfg/DFGLICMPhase.cpp 2015-05-02 17:33:36 UTC (rev 183718)
+++ trunk/Source/_javascript_Core/dfg/DFGLICMPhase.cpp 2015-05-02 18:36:24 UTC (rev 183719)
@@ -217,6 +217,43 @@
// we could still hoist just the checks.
// https://bugs.webkit.org/show_bug.cgi?id=144525
+ // FIXME: If a node has a type check - even something like a CheckStructure - then we should
+ // only hoist the node if we know that it will execute on every loop iteration or if we know
+ // that the type check will always succeed at the loop pre-header through some other means
+ // (like looking at prediction propagation results). Otherwise, we might make a mistake like
+ // this:
+ //
+ // var o = ...; // sometimes null and sometimes an object with structure S1.
+ // for (...) {
+ // if (o)
+ // ... = o.f; // CheckStructure and GetByOffset, which we will currently hoist.
+ // }
+ //
+ // When we encounter such code, we'll hoist the CheckStructure and GetByOffset and then we
+ // will have a recompile. We'll then end up thinking that the get_by_id needs to be
+ // polymorphic, which is false.
+ //
+ // We can counter this by either having a control flow equivalence check, or by consulting
+ // prediction propagation to see if the check would always succeed. Prediction propagation
+ // would not be enough for things like:
+ //
+ // var p = ...; // some boolean predicate
+ // var o = {};
+ // if (p)
+ // o.f = 42;
+ // for (...) {
+ // if (p)
+ // ... = o.f;
+ // }
+ //
+ // Prediction propagation can't tell us anything about the structure, and the CheckStructure
+ // will appear to be hoistable because the loop doesn't clobber structures. The cell check
+ // in the CheckStructure will be hoistable though, since prediction propagation can tell us
+ // that o is always SpecFinalObject. In cases like this, control flow equivalence is the
+ // only effective guard.
+ //
+ // https://bugs.webkit.org/show_bug.cgi?id=144527
+
if (readsOverlap(m_graph, node, data.writes)) {
if (verbose) {
dataLog(
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
