| Summary: | We should not employ the snippet code in the DFG if no OSR exit was previously encountered. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||
| Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, keith_miller, msaboff, saam | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Local Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
Created attachment 267301 [details]
proposed patch.
Created attachment 267302 [details]
x86_64 benchmark result.
With this change, we see the following progression in JSRegress: arguments-out-of-bounds 12.9152+-1.3205 ^ 10.2297+-0.3642 ^ definitely 1.2625x faster Comment on attachment 267301 [details]
proposed patch.
r=me
Thanks for the review. Landed in r194040: <http://trac.webkit.org/r194040>. Comment on attachment 267301 [details]
proposed patch.
This is the wrong policy. We usually only rely on exit sites as a last resort.
Why did you do this?
(In reply to comment #6) > Comment on attachment 267301 [details] > proposed patch. > > This is the wrong policy. We usually only rely on exit sites as a last > resort. > > Why did you do this? This was suggested by Geoff, and was implemented this way since the first op_sub snippet. The only issue was that I had a bug in how I implemented it. |
We already tried to do this but was doing it wrong. The is what we do now: if (Node::shouldSpeculateUntypedForArithmetic(leftChild.node(), rightChild.node()) || m_graph.hasExitSite(node->origin.semantic, BadType)) { fixEdge<UntypedUse>(leftChild); fixEdge<UntypedUse>(rightChild); ... The || should be a &&. We only want to employ the snippet code if we have actually encountered an OSR exit due to untyped operands not being supported.