Bug 91188 – DFG CFA may get overzealous in loops that have code that must exit

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

This is quite convoluted, but awesome.  Say that the CFA proves that some operation:

c: MyNode(@a, @b)

must exit because the predictions of @a and/or @b are incompatible with our implementation of NyNode.

But say that the CFA also proves that @a is Int32(0) and @b is Int32(1).  The CFA is very precise - often more precise than our prediction propagation.  So it could be that @a and @b have wacky predictions but can be proved to be something more specific.

And say that c:MyNode is in a loop, and that the result of @c feeds into @a because of an in-loop assignment to some variable.  But since the CFA proved that @c terminates, it ignored its output when computing the proof of @a.  If @c had executed, then @a would have been some variant of TOP.

Now you've got a situation where:

1) The CFA aggressively set the value of @a based on the assumption that @c terminates.
2) The fact that @a and @b were pinpointed to be values caused the backend, and subsequent executions of the CFA, to happily compile @c into a non-terminating instruction.

The result? You've now got a loop in which some variable appears to never be updated. This often manifests as a loop that runs forever because the variable in question is the loop's induction variable.

Furthermore, because of http://trac.webkit.org/changeset/119464, even if we fix this problem we're left with a deeper problem: CFG simplification may then lead OSR exit to believe that the body of the loop only sets but does not read the variable.  In fact it doesn't read it in the DFG code (since it's constant folded) but it does read it in the baseline code - so OSR must assume as such.  That's particularly important since this loop will OSR exit every time.

Fix and test on the way.

[Filip Pizlo]

Created attachment 152145 [details]
the patch

I will post the test shortly.  Still computing the expected result.

[Filip Pizlo]

Comment on attachment 152145 [details]
the patch

Ooops.  Never mind.

[Filip Pizlo]

Created attachment 152149 [details]
the patch

[Gavin Barraclough]

Comment on attachment 152149 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152149&action=review

> Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:60
> +            m_insertionSetForFolding.execute(*block);

This may insert the phantoms in the wrong place, Phi will fix.

[Filip Pizlo]

Created attachment 152152 [details]
the patch

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/122541