From 5cc94cc3937989e180c1c9897f15714534fc4d31 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Wed, 7 Mar 2018 17:04:18 +0000 Subject: Revert "Reland "make SkJumper stages normal Skia code"" This reverts commit 78cb579f33943421afc8423a39867fcfd69fed44. Reason for revert: lowp should be controlled by defined(JUMPER_IS_SCALAR), not defined(__clang__). So close. Original change's description: > Reland "make SkJumper stages normal Skia code" > > This is a reland of 22e536e3a1a09405d1c0e6f071717a726d86e8d4 > > Now with fixed #include paths in SkRasterPipeline_opts.h, > and -ffp-contract=fast for the :hsw target to minimize > diffs on non-Windows Clang AVX2/AVX-512 bots. > > Original change's description: > > make SkJumper stages normal Skia code > > > > Enough clients are using Clang now that we can say, use Clang to build > > if you want these software pipeline stages to go fast. > > > > This lets us drop the offline build aspect of SkJumper stages, instead > > building as part of Skia using the SkOpts framework. > > > > I think everything should work, except I've (temporarily) removed > > AVX-512 support. I will put this back in a follow up. > > > > I have had to drop Windows down to __vectorcall and our narrower > > stage calling convention that keeps the d-registers on the stack. > > I tried forcing sysv_abi, but that crashed Clang. :/ > > > > Added a TODO to up the same narrower stage calling convention > > for lowp stages... we just *don't* today, for no good reason. > > > > Change-Id: Iaaa792ffe4deab3508d2dc5d0008c163c24b3383 > > Reviewed-on: https://skia-review.googlesource.com/110641 > > Commit-Queue: Mike Klein > > Reviewed-by: Herb Derby > > Reviewed-by: Florin Malita > > Change-Id: I44f2c03d33958e3807747e40904b6351957dd448 > Reviewed-on: https://skia-review.googlesource.com/112742 > Reviewed-by: Mike Klein TBR=mtklein@chromium.org,herb@google.com,fmalita@chromium.org Change-Id: Ie64da98f5187d44e03c0ce05d7cb189d4a6e6663 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/112743 Reviewed-by: Mike Klein Commit-Queue: Mike Klein --- site/dev/contrib/jumper.md | 103 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 site/dev/contrib/jumper.md (limited to 'site/dev') diff --git a/site/dev/contrib/jumper.md b/site/dev/contrib/jumper.md new file mode 100644 index 0000000000..f7ea3b152a --- /dev/null +++ b/site/dev/contrib/jumper.md @@ -0,0 +1,103 @@ +Contributing to SkJumper +======================== + +SkJumper is the execution engine of SkRasterPipeline, a system we've been using +to accelerate CPU-bound work inside Skia, most notably color-space conversions +and color-correct drawing. + +(This is where I'd put my link to design document if I had one...) + +SkJumper is more annoying to contribute to than most Skia code because of its +offline compilation step. You'll need particular tools installed on your +machine and to tell GN about them. This document is designed to guide you +through this process and ease some of that annoyance. + +One-time Setup +-------------- + +To generate stage code you need Clang 5.0, objdump, and ccache. It's best that +Clang is exactly the same version we typically use (as of writing 5.0.0) and +you'll need objdump to be compiled with support for x86-64, ARMv7, and ARMv8. + +The easiest way to satisfy these contraints is to get your hands on a Mac and +install Xcode, Xcode command line tools, and [Homebrew](https://brew.sh). Once +you have `brew` installed, run these commands to get the tools you need: + + + + ls -d /usr/include >/dev/null || xcode-select --install + brew install llvm binutils ccache + +Setting up GN +------------------------- + +With your tools installed, tell GN about them + + skia_jumper_clang = path/to/clang-5.0 + skia_jumper_objdump = path/to/gobjdump + skia_jumper_ccache = path/to/ccache + +then regenerate and build as normal. + +If you look in your GN out directory, you should now see a bunch of `.o` files, +and `git status` should show no changes to `src/jumper/SkJumper_generated*.S`. +That's good. Those object files are the intermediates we parse to produce +the assembly files. We just leave them around in case you want to look at +them yourself. + +Make A Change +------------- + +Let's use the `from_srgb` stage as a little playground to make a real change. +Linearizing sRGB encoded bytes is slow, so let's pretend we've decided to trade +quality for speed, approximating the existing implementation with a simple square. + +Open up `SkJumper_stages.cpp` and find the `from_srgb` stage. It'll look like + + + + STAGE(from_srgb) { + r = from_srgb(r); + g = from_srgb(g); + b = from_srgb(b); + } + +Let's replace whatever's there with our fast approximation: + + + + STAGE(from_srgb) { + r *= r; + g *= g; + b *= b; + } + +When you save and re-Ninja, you should now see changes to +`src/jumper/SkJumper_generated.S` and `src/jumper/SkJumper_generated_win.S`. +If you can't read assembly, no big deal. If you can, run `git diff`. You +should see the various `sk_from_srgb_*` functions get dramatically simpler, +something like three multiplies and a couple other bookkeeping instructions. + +It's not unusual for isolated changes in one stage to cause seemingly unrelated +changes in another. When adding or removing any code you'll usually see all +the comments in branch instructions change a little bit, but the actual +instruction on the left won't change. When adding or removing uses of +constants, you'll often see both the comment and instruction on the left change +for other loads of constants from memory, especially on x86-64. You'll also +see some code that looks like garbage change; those are the constants. If +any of this worries you, please do go running to someone who knows more for +help, but odds are everything is fine. + +At this point things should just be business as usual. Any time you change +`SkJumper_stages.cpp`, Ninja ought to notice and regenerate the assembly files. + +Adding a new Stage +------------------ + +Adding a new stage is a lot like changing an existing stage. Edit +`SkJumper_stages.cpp`, build Skia, test, repeat until correct. + +You'll just need to also edit `SkRasterPipeline.h` to add your new stage to the +macro listing all the stages. The stage name is the handle normal Skia code +uses to refer to the stage abstractly, and the wiring between +`SkRasterPipeline::foo` and `STAGE(foo) { ... }` should work automatically. -- cgit v1.2.3