Bug 151415 - Remove some unnecessary jumps in snippet code.
Summary: Remove some unnecessary jumps in snippet code.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on: 151440
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-18 15:32 PST by Mark Lam
Modified: 2015-11-19 10:02 PST (History)
4 users (show)

See Also:


Attachments
proposed patch. (7.95 KB, patch)
2015-11-18 15:41 PST, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff
x86_64 benchmark result. (64.45 KB, text/plain)
2015-11-18 15:42 PST, Mark Lam
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2015-11-18 15:32:42 PST
Previously, the snippet generators always emit a jump at the end of the fast path.  In the baseline JIT and FTL, this results in a jump to the very next instruction.  I've change it to assume that the fast path will just fall thru, and let the client decide instead if it wants/needs a jump or not after the fast path.

I also changed the generators to provide a didEmitFastPath() query explicitly declare if they actually generated the fast path, instead of having the client infer it from an empty endJumpList.
Comment 1 Mark Lam 2015-11-18 15:41:47 PST
Created attachment 265791 [details]
proposed patch.
Comment 2 Mark Lam 2015-11-18 15:42:53 PST
Created attachment 265792 [details]
x86_64 benchmark result.
Comment 3 Geoffrey Garen 2015-11-18 15:55:46 PST
Comment on attachment 265791 [details]
proposed patch.

r=me

Small tweaks to snippet code generation might be nice -- but as you can see from benchmark results, they don't influence the bottom line. It's much more valuable to move forward with more snippet coverage, since that will influence the bottom line.
Comment 4 Mark Lam 2015-11-18 16:31:37 PST
Thanks for the review.  For the record, this work came out of investigating some baseline perf regressions in the op_mul snippet work: https://bugs.webkit.org/show_bug.cgi?id=151393.  I just updated op_add and op_sub here to match and ran the benchmarks for a sanity check.

Landed in r192599: <http://trac.webkit.org/r192599>.
Comment 5 Mark Lam 2015-11-19 10:02:35 PST
This patch exposed a bug: https://bugs.webkit.org/show_bug.cgi?id=151445.  The fix for that landed in r192632.