Commit Graph

70 Commits

Author SHA1 Message Date
Mike Klein
d48488b5ea reorder to minimize register pressure
Rewrite program instructions so that each value becomes available as
late as possible, just before it's used by another instruction.  This
reorders blocks of instructions to reduce them number of temporary
registers in flight.

Take this example of the sort of program that we naturally write,
noting the registers needed as we progress down the right:

    src = load32 ...          (1)
    sr = extract src ...      (2)
    sg = extract src ...      (3)
    sb = extract src ...      (4)
    sa = extract src ...      (4, src dies)

    dst = load32 ...          (5)
    dr = extract dst ...      (6)
    dg = extract dst ...      (7)
    db = extract dst ...      (8)
    da = extract dst ...      (8, dst dies)

    r = add sr dr             (7, sr and dr die)
    g = add sg dg             (6, sg and dg die)
    b = add sb db             (5, sb and db die)
    a = add sa da             (4, sa and da die)

    rg   = pack r g ...       (3, r and g die)
    ba   = pack b a ...       (2, b and a die)
    rgba = pack rg ba ...     (1, rg and ba die)
    store32 rgba ...          (0, rgba dies)

That original ordering of the code needs 8 registers (perhaps with a
temporary 9th, but we'll ignore that here).  This CL will rewrite the
program to something more like this by recursively issuing inputs only
once needed:

    src = load32 ...       (1)
    sr  = extract src ...  (2)
    dst = load32 ...       (3)
    dr  = extract dst ...  (4)
     r  = add sr dr        (3, sr and dr die)

    sg  = extract src ...  (4)
    dg  = extract dst ...  (5)
     g  = add sg dg        (4, sg and dg die)

    rg  = pack r g         (3, r and g die)

    sb  = extract src ...  (4)
    db  = extract dst ...  (5)
     b  = add sb db        (4, sb and db die)

    sa  = extract src ...  (4, src dies)
    da  = extract dst ...  (4, dst dies)
     a  = add sa da        (3, sa and da die)

    ba  = pack b a         (2, b and a die)

    rgba = pack rg ba ...  (1, rg and ba die)
    store32 rgba  ...      (0)

That trims 3 registers off the example, just by reordering!
I've added the real version of this example to SkVMTest.cpp.
(Its 6th register comes from holding the 0xff byte mask used
by extract, in case you're curious).

I'll admit it's not exactly easy to work out how this reordering works
without a pen and paper or trial and error.  I've tried to make the
implementation preserve the original program's order as much as makes
sense (i.e. when order is an otherwise arbitrary choice) to keep it
somewhat sane to follow.

This reordering naturally skips dead code, so pour one out for ☠️ .
We lose our cute dead code emoji marker, but on the other hand all code
downstream of Builder::done() can assume every instruction is live.

Change-Id: Iceffcd10fd7465eae51a39ef8eec7a7189766ba2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/249999
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-10-22 21:49:05 +00:00
Mike Klein
97afd2e21c add bsl.16b, cmeq.4s, cmgt.4s
These implement select, eq_i32, lt_i32, and gt_i32 on ARMv8.

Change-Id: Ic36dda1cc425ca91700f9b120594e420ea0f560a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248970
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-10-17 01:54:33 +00:00
Mike Klein
0f61c12737 add used_in_loop bit to skvm::Builder::Instruction
Most hoisted values are used in the loop body (and that's really the
whole point of hoisting) but some are just temporaries to help produce
other hoisted values.  This used_in_loop bit helps us distinguish the
two, and lets us recycle registers holding temporary hoisted values not
used in the loop.

The can-we-recycle logic now becomes:
   - is this a real value?
   - is it time for it to die?
   - is it either not hoisted or a hoisted temporary?

The set-death-to-infinity approach for hoisted values is now gone.  That
worked great for hoisted values used inside the loop, but was too
conservative for hoisted temporaries.  This lifetime extension was
preventing us from recycling those registers, pinning enough registers
that we run out and fail to JIT.

Small amounts of refactoring to make this clearer:
   - move the Instruction hash function definition near its operator==
   - rename the two "hoist" variables to "can_hoist" for Instructions
     and "try_hoisting" for the JIT approach
   - add ↟ to mark hoisted temporaries, _really_ hoisted values.

There's some redundancy here between tracking the can_hoist bit, the
used_in_loop bit, and lifetime tracking.  I think it should be true, for
instance, that !can_hoist && !used_in_loop implies an instruction is
dead code.  I plan to continue refactoring lifetime analysis (in
particular reordering instructions to decrease register pressure) so
hopefully by the time I'm done that metadata will shake out a little
crisper.

Change-Id: I6460ca96d1cbec0315bed3c9a0774cd88ab5be26
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248986
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-10-16 18:29:06 +00:00
Mike Klein
b5a30767e0 Reland "mark which SkVM tests should JIT or not"
This is a reland of 52435503e9

with better checks for when we should expect JIT and not.

Original change's description:
> mark which SkVM tests should JIT or not
>
> Most of these tests converted over to test_interpreter_only()
> are failing to JIT because of unimplemented instructions.  No
> bug there, just TODOs.
>
> But SkVM_hoist _should_ be JITting.  A while back I landed a CL
> that messed with value lifetimes that prevents it from JITting.
> Will be using this as a regression test to fix that bug.
>
> Change-Id: Id2034f6548a45ed9aeb9ae3cbb24d389cad7dc60
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248980
> Commit-Queue: Mike Klein <mtklein@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: Mike Klein <mtklein@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Herb Derby <herb@google.com>

Cq-Include-Trybots: skia.primary:Test-Android-Clang-NVIDIA_Shield-CPU-TegraX1-arm64-Release-All-Android,Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-SK_CPU_LIMIT_SSE2,Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-SK_CPU_LIMIT_SSE41,Test-Mac10.13-Clang-VMware7.1-CPU-AVX-x86_64-Debug-All-NativeFonts,Test-Mac10.14-Clang-VMware7.1-CPU-AVX-x86_64-Debug-All-NativeFonts
Change-Id: Id7bde7e879649e435fa424a9c9d6c51a31afd5e9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248990
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-10-16 17:35:06 +00:00
Mike Klein
4e11526e3d Revert "mark which SkVM tests should JIT or not"
This reverts commit 52435503e9.

Reason for revert: lots of bots can't JIT, duh...

Original change's description:
> mark which SkVM tests should JIT or not
> 
> Most of these tests converted over to test_interpreter_only()
> are failing to JIT because of unimplemented instructions.  No
> bug there, just TODOs.
> 
> But SkVM_hoist _should_ be JITting.  A while back I landed a CL
> that messed with value lifetimes that prevents it from JITting.
> Will be using this as a regression test to fix that bug.
> 
> Change-Id: Id2034f6548a45ed9aeb9ae3cbb24d389cad7dc60
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248980
> Commit-Queue: Mike Klein <mtklein@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: Mike Klein <mtklein@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Herb Derby <herb@google.com>

TBR=mtklein@google.com,herb@google.com,ethannicholas@google.com

Change-Id: Ieea4b06f0d32249e3da56c6810d3c45c2abf2689
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248989
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-10-16 16:48:58 +00:00
Mike Klein
52435503e9 mark which SkVM tests should JIT or not
Most of these tests converted over to test_interpreter_only()
are failing to JIT because of unimplemented instructions.  No
bug there, just TODOs.

But SkVM_hoist _should_ be JITting.  A while back I landed a CL
that messed with value lifetimes that prevents it from JITting.
Will be using this as a regression test to fix that bug.

Change-Id: Id2034f6548a45ed9aeb9ae3cbb24d389cad7dc60
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/248980
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-10-16 15:45:16 +00:00
Mike Klein
6b4143e11f move skvm debug tools back to core
I happened to have this on when profiling skottie_tool and got curious
why I was seeing the interpreter run and not JIT code.  Mostly this
moves the code in bulk out of SkVMTest.cpp to SkVM.cpp so that code in
SkVM.cpp can call dump() on itself.

Also this CL has the skvm::Program hang onto the original value-based
builder program (in addition to its own interpreter program and JIT
program if we can).  This is entirely so that when JIT bails out I
can have it dump out both the builder and interpreter programs for
more debugging aid.

I'm still going to need more debug tools somewhere to figure out
what the program that needs 17 registers is, and what to do about
it.

Finally, remove skvmtool.  It's annoying to maintain its build
rules, and I don't use it much if ever anymore.

Change-Id: I995d15d04bda79ddfc4d68bda8aaa3b5b9261f08
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/242520
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-09-18 19:41:11 +00:00
Mike Klein
f996311003 extend lifetimes for hoisted used in loop
This makes the register recycling checks a bit more
precise.  At head we never recycle a register that's
holding a hoisted value, which is overly conservative.

We really should never recycle a register that's still
needed.  By extending the lifetime of any hoisted value
that's used in the loop, we prevent that, while still
allowing hoisted values that are only used in hoisted
computation to be reused.

This takes just a small tweak in the JIT code (removing
the !hoisted({x,y,z}) checks), and a somewhat larger
refactoring in the interpreter, making both hoisted and
non-hoisted code go through the same recycling register
assignment flow.

There's one diff in the existing cases where we now
reuse a hoisted register, and I've added a second test
just to make sure it's covered explicitly.

Change-Id: I25b37ab1f1fea3042d7fd167529abc8fed1dddff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/233239
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-08-13 02:08:16 +00:00
Mike Klein
b994412823 select, {eq,lt,gt}_i32 on x86
Add vpblendvb, vpcmpeqd, and vpcmpgtd, to implement select and eq/lt/gt.
I want to think just a touch bit more about neq, lte, and gte.

This is enough to JIT everything SkVMBlitter creates today.

There are 24 possible argument orders to vpblendvb,
so I'm sure I've got them wrong somehow, even with the new test.

Change-Id: I357664b866d8258a2b5438d520f47542ad581c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/232060
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-08-02 18:28:15 +00:00
Mike Klein
95529e8216 x86 store16
Nothing particularly tricky.  Very much like store8,
but with one fewer shuffle.

This lets some of the 565 blitters JIT!

Change-Id: I853905bda30a0cda89f3fcb5fef1dfe62725063b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/232059
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-08-02 17:47:34 +00:00
Mike Klein
52010b7dd8 x86, load16
Very similar to load8.

The only interesting thing is how different vpinsrw is from vpinsrb (and
vpinsrd and vpinsrq)... different map of instructions entirely.

Change-Id: Ia413b83604dd2d277d59495c5f693f505c35be9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/232058
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-08-02 16:52:44 +00:00
Mike Klein
94d054b53c x86 uniform8
Add vbroadcastss(Ymm, Xmm) and expand movzbl() to support an offset.

The sequence

    movzbl          (load a byte, zero-extend to 4 byte)
    vmovd           (move that 4 byte value to an xmm)
    vbroadcastss    (broadcast low 4 bytes of xmm to all lanes of ymm)

implements uniform8.

Change-Id: I1d3125920d19dcb3cad5980495310bc95b9dffee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/232057
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-08-02 16:41:44 +00:00
Mike Klein
788967eb37 add vbroadcastss(Ymm, GP64, int)
vbroadcastss with a register argument and immediate offset implements
uniform32.  And this turns on the JIT for some new SkVMBlitter paths
that pass more arguments that I'd previously wired up, so add a few
more.

Change-Id: I66db1286dcdb2c4a4ba7c43f2dc2cd13564d4d34
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/232056
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-08-02 16:07:37 +00:00
Mike Klein
9fdadb949f test a (the) zero-arg program
Just because a program doesn't read from or
write to memory doesn't mean it's pointless.

Oh wait, yes it does.  It shouldn't crash though.

Change-Id: I6a9c26c065831f9598afccce6e0a34a178cbd925
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/230839
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-30 19:58:37 +00:00
Mike Klein
d4546d6c4c add vmovdqa(ymm,ymm)
Fills in a little TODO.
Also add assembler unit tests for similar float->int and int->float.

Tested by SkVM_mad, SkVM_madder.

Change-Id: I5334029927fdecb0ff7f5a3b081cf2ce7b23995c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/230838
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-30 18:43:35 +00:00
Mike Klein
81d526703c more unit tests
Backfill tests for some uncovered operations.

Mostly functional tests, with some targeted at particular
corners of code generation, and a few assembler unit tests.

This doesn't get up to 100% line coverage, but I think it
covers all the non-failure cases.

Change-Id: Ib701df0c505aa41e3b2fe3cd429447acb294b752
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/230837
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-30 17:35:15 +00:00
Mike Klein
5591fdf930 small refactors
Change-Id: I58b52d3e1d05d0834be30e00d991636e227cbf0b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/230836
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-30 15:43:13 +00:00
Mike Klein
8ac9f4e5b2 flesh out SkVM ops a bit more
Add missing comparison and selection ops, bit casts, 16-bit memory
operations, gathers, uniform loads, and fill in math holes where
reasonable.  Update some names to be a bit more regular.

I think all instructions are implemented in the interpreter,
and many tested.  More testing and JITs to follow.

Change-Id: I8cf377e8b72a86ac950e020892ce82b39e9d7277
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229893
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-29 20:43:10 +00:00
Mike Klein
f98d0d31c4 let JIT code hoist when possible
This first tries to JIT while hoisting all constants,
and if that fails, tries again hoisting no constants.

I figure this is one of those 80/20 deals for how to
handle constant hoisting and register pressure.  This
probably mostly moots doing anything fancy like using
memory operands with AVX or lane operands with NEON.

This _doesn't_ moot hoisting the NEON tbl arguments,
which is not yet done here, but probably my next CL.

Change-Id: Id09d5cdddcdb45207bdfc914a5a3128a481a26f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229058
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-22 21:06:34 +00:00
Mike Klein
5e533c9e1f move hoist analysis back into Builder
Even if a JIT ultimately doesn't end up hoisting any values, it's going
to want this information while it decides.  Writing it in one place also
ensures we only get it wrong in one place...

I'm no_ extending the lifetime of hoisted instructions here in Builder.
That's something to leave to the backend so they have the flexibility of
which of these values to hoist, if any.  If they don't hoist, they'll
need to know when the value dies.

Moving this information back here lets the test expectation goldens
reflect the hoist bit again too.  Kind of nice.

Change-Id: Ib165ca898a97c1d822cb28fe24f15bae4d570a17
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229024
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-22 19:34:06 +00:00
Mike Klein
4a13119a60 always fma in mad_f32()
We can always move data around so that an FMA is possible using no more
registers than we would otherwise, and on x86, evne using no more
instructions.

The basic idea here is that if we can't reuse one of the inputs to
destructively host the FMA instruction, the next best thing is to copy
one of the arguments into tmp() and accumulate the FMA there.

Once the FMA has happened, we just need to copy that result to dst().
We can of course skip that copy if dst() == tmp().  On x86 we never need
that copy; dst() and tmp() are picked using the same logic except that
dst may alias one of its inputs, and we only fall into this case after
we've already found it doesn't.  So we can just assert dst() == tmp()
rather than check it like we do on ARM.

It's subtle, but I think sound.

I'm using logical-or to copy registers around.  This is a little lazy,
but maybe not as lazy as it looks: on ARM that is _the_ way to copy
registers.  There's a vmovdqa instruction I could use on x86, TBD.

All paths through this new code were being exercised on ARM, but we
didn't have anything hitting the tmp case on x86, so I've added a new
unit test that hits the corner cases of both implementations.

Change-Id: I5422414fc50c64d491b4933b4b580b784596f291
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228630
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-19 20:12:42 +00:00
Mike Klein
1326749832 add sli.4s, use it in pack sometimes
We have pack(x,y,imm) = x | (y<<imm) assuming (x & (y<<imm)) == 0.

If we can destroy x, sli (shift-left-insert) lets us implement that
as x |= y << imm.  This happens quite often, so you'll see sequences
of pack that used to look like this

	shl	v4.4s, v2.4s, #8
	orr	v1.16b, v4.16b, v1.16b
	shl	v2.4s, v0.4s, #8
	orr	v0.16b, v2.16b, v3.16b
	shl	v2.4s, v0.4s, #16
	orr	v0.16b, v2.16b, v1.16b

now look like this

	sli	v1.4s, v2.4s, #8
	sli	v3.4s, v0.4s, #8
	sli	v1.4s, v3.4s, #16

We can do this thanks to the new simultaneous register assignment
and instruction selection I added.  We used to never hit this case.

Change-Id: I75fa3defc1afd38779b3993887ca302a0885c5b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228611
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-19 18:37:12 +00:00
Mike Klein
9e2218a06a restore aarch64 JIT
Trying to keep most of the structural parts shared between x86_64 and
aarch64.  Not sure if this will stay factored like this long-term, but
the last version felt like there was a bit too much redundancy, and I
don't want to write things like register management more often than have
to.

Change-Id: Ieeb21f433715a730c41c85d657c5b33fa4702696
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228608
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-19 18:06:52 +00:00
Mike Klein
237dbb4d87 small cleanups
- get rid of variadic Assembler::byte()... not used very often
  - rename Assembler::byte(ptr,n) to bytes()
  - align with 0 bytes, get rid of nop()

Change-Id: I7564d3bad00e3f0d1c7a80153c445966914fccf0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228601
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-19 15:36:11 +00:00
Mike Klein
37607d4ccd Reland "more JIT refactoring"
This is a reland of 558b639225

PS2... oh, right, not everything supports AVX2.

Original change's description:
> more JIT refactoring
>
> This re-enables AVX2 JIT with simultaneous register assignment and
> instruction selection.  You can see it working in a very basic way in
> how we choose instructions and registers for Op::mad_f32.
>
> Constants are still broadcast, here inside the loop instead of hoisted.
> I think it'll probably end up best to use constants directly from memory
> (as in vpshufb's masks), falling back to these in-loop broadcasts when
> that can't work.
>
> Change-Id: If17d51b9960f08da3612e51ac04424e996bf83d4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228366
> Commit-Queue: Mike Klein <mtklein@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>

Cq-Include-Trybots: skia.primary:Test-Mac10.13-Clang-VMware7.1-CPU-AVX-x86_64-Debug-All-NativeFonts
Change-Id: I6f99d275040abe6210a980fc544f7f22c3b85727
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228476
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-18 23:59:47 +00:00
Mike Klein
d864d1dc19 Revert "more JIT refactoring"
This reverts commit 558b639225.

Reason for revert: broke some perf bots

Original change's description:
> more JIT refactoring
> 
> This re-enables AVX2 JIT with simultaneous register assignment and
> instruction selection.  You can see it working in a very basic way in
> how we choose instructions and registers for Op::mad_f32.
> 
> Constants are still broadcast, here inside the loop instead of hoisted.
> I think it'll probably end up best to use constants directly from memory
> (as in vpshufb's masks), falling back to these in-loop broadcasts when
> that can't work.
> 
> Change-Id: If17d51b9960f08da3612e51ac04424e996bf83d4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228366
> Commit-Queue: Mike Klein <mtklein@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>

TBR=mtklein@google.com

Change-Id: Id6cd5acd873499bb394009489d77e7636ecbc9c6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228462
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-18 23:21:24 +00:00
Mike Klein
558b639225 more JIT refactoring
This re-enables AVX2 JIT with simultaneous register assignment and
instruction selection.  You can see it working in a very basic way in
how we choose instructions and registers for Op::mad_f32.

Constants are still broadcast, here inside the loop instead of hoisted.
I think it'll probably end up best to use constants directly from memory
(as in vpshufb's masks), falling back to these in-loop broadcasts when
that can't work.

Change-Id: If17d51b9960f08da3612e51ac04424e996bf83d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228366
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2019-07-18 22:24:42 +00:00
Mike Klein
62bccdaf3c move death back into Builder::Instruction
I find myself passing around parallel vectors of Builder::Instructions
and deaths so often that it just makes more sense practically to store
them together.  It's a little awkward that the values are only useful
after calling done(), but I can live with that.

Get a little more careful about mutation, passing Builder::Instructions
by const&.   Instead of extending lifetimes of live hoisted
instructions, just check for them in maybe_recycle_register() instead.

Change-Id: I1cb9e25c1a7c46a250c2271334821be8535353bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228367
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-18 19:01:44 +00:00
Mike Klein
c2fb3b4b72 split deaths() out of other analysis
I'm slowly refactoring my way to where hoisting and register assignment
are done in backend-specific ways, but this liveness analysis is always
going to be useful for each backend.

Use deaths() to restore friendly ☠️  dead code markers in test dumps.

Change-Id: I3ab94665bbbbf0788b0b27e00d644eba927dff47
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228113
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2019-07-17 18:11:10 +00:00
Mike Klein
9977efa703 test both JIT and interpreter
Add Program::dropJIT() to allow us to proactively drop
any JIT code forcing fallback on the interpreter,
and use it to test both on JIT-supported platforms.

Other platforms will just test the interpreter twice.

Change-Id: I607d00ef3c648e66a0b3a1374b11aa82dbfff70c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/227424
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-07-16 22:31:17 +00:00
Mike Klein
2616efda08 pin down arg() stride (a.k.a. type) info sooner
Arg strides are the reason JIT happens lazily in Program::eval() today
instead of proactively in Builder::done() or Program's constructor.  It
also just really doesn't make sense to delay this information... it's
not like you can change it up sanely between calls to eval().

The argument index now comes implicitly from the order of calling arg().
This may seem logically independent, but it prevents a weird situation
where you could use the same argument index twice with different
strides... not sure what that would mean.

Change-Id: I0f5d46e94a1ca112a72675c5492f17c0dd825ce0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/227390
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-16 21:06:45 +00:00
Mike Klein
35b97c3130 handle x86 tail in JIT code too
Basically the same deal as aarch64:

    - a bunch of instructions to rewrite control
      flow to be two loops, body and tail

    - a bunch of instructions to support scalar
      loads and stores in the tail

We can now remove the JIT::mask field.

I've removed the SkUNREACHABLE I'd put in for the ARM code...  as
written the interpreter is still reachable by the loser if two threads
race to JIT the program.  Medium term I plan to move JIT compilation to
a more proactive time, eliminating the need for the lock and letting the
interpreter become truly unreachable.

I had a little bit of a false start with what instructions to use for
scalar load8 and store8, first starting with instructions that loaded
via GP registers, then remembering vpinsrb and vpextrb can take a memory
argument, loading into xmm directly.  I've left the first instructions I
used in the file, still implemented but only used from the unit tests.
They're pretty common and will probably be useful some day.

Change-Id: I471b13026af4b1c6e861a53159f9df5f0285447c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/227178
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-15 03:13:56 +00:00
Mike Klein
86a645c5d9 fix bug for add/sub with r8-r15
I had been setting the REX R bit to select high registers,
but you actually set the B bit.  Don't know how I got that
wrong before... the leading byte should be 49 not 4c.

    $ cat test.s
    foo:
        addq $7, %r8

    $ clang -c test.s && objdump -d test.o

    0000000000000000 <foo>:
       0:	49 83 c0 07          	add    $0x7,%r8

Change-Id: I039e1c4f4ea20523a1e2cc9bcf5f6d9321a6223b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/227177
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-15 02:04:36 +00:00
Mike Klein
65c10b5018 make all instructions take two-way labels
This turned out to be quite an easy transformation
with yesterday's work already done.  No codegen changes.

Change-Id: Ife19ab7731514c54cfed963a6d2e9b1ec2246997
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/227137
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-07-12 15:56:06 +00:00
Mike Klein
ce7b88ce7d bidirectional Labels
Support forward references in Label.

In addition to tracking the current Label offset (used for
backward references essentially just the same as before this CL)
we also store a list of instructions that refer to each Label.
When a Label moves, each instruction gets a new displacement.

To make this a little easier, remove the 8-bit jump form on x86...
this way all x86 displacements are 32-bit and and all ARM 19-bit.

For now only cbz() supports this, just to start somewhere.
More to do but it's worth an early design review.

Change-Id: I23d2bcd7742965ab694ae4828f53409cb9fc807f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/226937
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-07-12 12:53:24 +00:00
Mike Klein
4cfe3ed0f2 instructions for JIT tail support on ARM
This adds a bunch of instructions we'll need to handle the N < 4 tail
within the JIT code on ARM.

   - ldrb/strb are 1-byte load and stores
   - sub subtracts without setting flags
   - cmp just sets flags (actually just subs with an xzr destination)
   - add b and b.lt, just like b.ne
   - cbz and cbnz... we only need cbz but I accidentally did cbnz first

Once I add support for forward jumps, we'll be able to use these
instructions to restructure the loop to

    entry:
        hoisted setup
    loop:
        if N < 4, jump tail      (cmp N,#4; b.lt tail)
        ... handle 4 values ...
        jump loop                (b loop)
    tail:
        if N == 0, jump end      (cbz N, end)
        ... handle 1 value ...
        jump tail                (b tail)
    end:
        ret

Change-Id: I62d2d190f670f758197a25d99dfde13362189993
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/226828
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-11 17:49:44 +00:00
Mike Klein
aab45b5638 add misc. value programs to SkVMTest.expected
Noticed we were only dumping the final register
programs for the integer code.  Might as well also
track the value programs.

Change-Id: I417c5c655b632691557bbbb136dcbd3f3167af9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/225324
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-07-02 23:13:06 +00:00
Mike Klein
7e65076ae3 move Builder/Program dump()
This is test-only code only used by SkVMTest.cpp,
so it can live there.  This cuts the dependency
of SkVM on SkStream and co.

Change-Id: I7695e527b2d16e4485f8c5f4cd39bb8300e9221d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/225321
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-02 23:10:23 +00:00
Mike Klein
1fa149a713 finish up arm64 ops
Some small refactoring to common up redundant opcode building.

Oddly, I think I've got better codegen than what Clang would do here.
Clang doesn't generate uxtl-based code to unpack 8-bit to 32-bit,
instead preferring to load each byte one at a time and insert them one
at a time.

Me:
    ldr  s0, [x0]
    uxtl v0.8h, v0.8b
    uxtl v0.4s, v0.8h

Clang:
    ldrb  w8,  [x0]
    ldrb  w9,  [x0, #1]
    ldrb  w10, [x0, #2]
    ldrb  w11, [x0, #3]
    fmov  s0,      w8
    mov   v0.s[1], w9
    mov   v0.s[2], w10
    mov   v0.s[3], w11

Change-Id: I0fdf5c6cdcde6a4eb9290936284fd3ffcb2159f6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/224821
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-07-01 17:26:03 +00:00
Mike Klein
e51632e8c5 128-bit load / store
Change-Id: I665f43279ac7bfd3dffbf38a10a571b959a3425c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223977
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-26 22:56:51 +00:00
Mike Klein
15a368d519 some scalar ops
Change-Id: I1603f89603755914ed91645a2e5eeb9c87f63bbe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223929
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-26 21:33:29 +00:00
Mike Klein
65809146b2 more aarch64 instructions
Change-Id: Iabb9a2357b9279ab6e5a3ee899c5d029d35499e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223697
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-25 19:33:53 +00:00
Mike Klein
9f4df80fda baby steps for aarch64 support
So far this is just as easy as I had hoped.

Change-Id: I5f69a900b32d9bf70156b55e334233d7376b820f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223340
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-25 13:40:49 +00:00
Mike Klein
88c0a90ee5 assemble directly into mmap'd memory
Instead of allocating into a std::vector, we do one quick first pass to
measure how much memory we need to allocate, mmap enough pages for that,
then another real writing pass.

This cuts a microsecond or so off the profile.  There's another
microsecond left to cut if we could eliminate that first measuring pass,
but I'm no longer sure it's easy to come up with a good upper limit on
the program size now that I'm thinking about the data part of the
program as well.

vpshufb is our current max instruction at 9 bytes of code, but that also
implies another 32 bytes of control data.  I'm not sure I feel very
clever allocating 41 * |instructions| bytes to be conservatively safe...
it seems like ridiculous overkill.

Ultimately I found it easier to just measure twice, cut once.

Change-Id: I16ccdafbc789711837b41b3d5a557808798eb1b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223305
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-25 03:02:28 +00:00
Mike Klein
2b7b2a2331 add bit_clear
I was just reading the ARM docs and realized that their BIC ("BIt
Clear") is the same as SSE's ANDN ("AND Not") instruction.  It's kind of
a neat little tool to have laying around... comes up more than you'd
think, and it's sometimes the clearest way to express what you're doing,
as in the changed program here where the comment is "mask away the low
bits".  That's a bit_clear with a mask for what you want to clear away!

And the real reason to write this up is that I want to have a CL to
point to that shows how to add an instruction top to bottom.

Change-Id: I99690ed9c1009427b3986955e7ae6264de4d215c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223120
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Mike Reed <reed@google.com>
2019-06-24 16:31:15 +00:00
Mike Klein
6bbeb4ab72 remove xbyak
For now, disable the vpmovusdb AVX-512 instruction, using the compound
AVX2 fallback instead.  I need to learn how to encode EVEX prefixes
before we can use that, and it's not very important.

That's everything!  We're fully in control now, and should be able to
run this on any x86-64 Linux or Mac.  And we can relax some of the
defined(SKVM_JIT) guards so that, e.g., we can unit test Assembler even
on all platforms.

Stifle some warnings about ~bool by ~(int)bool.

Would like to enable when is_mac too but can't seem to get past
(bogus?) thread annotation on the bots.  My local Mac is fine. :/

Change-Id: If00bdd97ebd9684ed109933e2fa70c5e6f6ea339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222631
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-22 00:16:38 +00:00
Mike Klein
f3881b278e vmovq
Change-Id: Id83573bb7e66c0a6316917ae17abe7f56a172941
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222629
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-21 22:01:14 +00:00
Mike Klein
ae51aa3adf vpmovzxbd
Change-Id: I3ae8b3a6a109f6286dfc28a1c2b8e9aea913bcae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222626
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-21 21:52:54 +00:00
Mike Klein
120d9e8e18 vmovups, both ways
Change-Id: I6790b59df1e4b4ee9b36b24819586411eff6324d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222623
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-21 21:47:44 +00:00
Mike Klein
060eaaaaef jne
Change-Id: Ie6b033367e24ae92f6e246963f1f014c4d7cf013
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222859
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-21 21:05:27 +00:00