Commit Graph

222 Commits

Author SHA1 Message Date
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
Mike Klein
04db9c265a vpshufb
Change-Id: If23681e7a34a091cb78e5bd469d71c56b9cf5dc8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222858
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-21 19:52:48 +00:00
Mike Klein
e505341a24 vbroadcastss
This shows off a little how easy backwards-only labels are.

The rip == rbp + Mod::Indirect convention isn't something
you'd be able to guess without just looking at the docs.

I'm not actually sure if you can only use rbp or also r13,
but LLVM seems to always do the equivalent of rbp... might
just be that high bit in VEX is ignored: they're registers
5 and 13, 8 apart, only distinguished by that bit.

Convenienly RIP addressing is always 32-bit, so there's
no benefit to spending time checking whether the offset
fits in a byte, though most of our offsets would.

Change-Id: I01b7fb1500667e1bf98490d5144459f92e1b375d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222857
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-21 19:28:37 +00:00
Mike Klein
62392726b7 rearrange code,data -> data,code
By putting data first in descending alignment then code, we never need
any alignment padding.

This also makes all jumps and ip-relative data loads backward, so
they're really easy to assemble.  No need for any sort of deferred
where-does-this-label-mean logic; the label can just be a simple byte
offset established before you need to use it.

Nothing new switched off of Xbyak in this CL, but the rearrangement
makes the rest a lot easier.

The one downside I've found so far is that the disassembly of the
first instruction can get confused into data or other instructions,
e.g.

  63:   01 ff                           add    %edi,%edi
  65:   00 ff                           add    %bh,%bh
  67:   00 00                           add    %al,(%rax)
  69:   ff 00                           incl   (%rax)
  6b:   ff c4                           inc    %esp
  6d:   e2 7d                           loop   ec <skvm-jit-884702985+0xac>
  6f:   18 05 eb ff ff ff               sbb    %al,-0x15(%rip)        # 60 <skvm-jit-884702985+0x20>
  75:   c4 e2 7d 18 0d e6 ff ff ff      vbroadcastss -0x1a(%rip),%ymm1        # 64 <skvm-jit-884702985+0x24>
  7e:   c4 e2 7d 18 15 e1 ff ff ff      vbroadcastss -0x1f(%rip),%ymm2        # 68 <skvm-jit-884702985+0x28>

There are 3 vbroadcastss instructions here, each starting with c4 e2 7d
18, but the first has been disassembled as if its c4 were part of the
last data entry (0xff00ff00) as inc %esp.

Probably not a big deal for now, particularly since those vbroadcastss
are all outside the loop and never show up on a profile.  If it gets too
confusing I think we can dump the programs starting from the beginning
of the code instead of from the data; we won't be able to inspect the
data, but everything should disassemble perfectly.

Change-Id: I0cc864359fd0740fc026070eaf2b6cb130783a57
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222574
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-21 15:38:41 +00:00
Mike Klein
ff0ae81e74 two register + immediate ops
The encoding kind of all goes through the same paths,
as the three argument instructions, but like the nursery
rhyme when there are only two they kind of all roll over
and the op-extension hops into the bed.

vpermq is the first place we need to set the W bit
to indicate a 64-bit lane operation, so a little
minimal plumbing for that.  It takes its arguments
a little differently too, passing dst where you'd
expect, the source where we'd pass y, and requiring
us to pass literal 0000 for the vvvv bits in VEX
(inverted as normal to literal 1111).

Change-Id: I91a4cd1b316eb908992631ce8b2cb3c62078e8c6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222565
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-20 21:25:16 +00:00
Mike Klein
397fc88fc0 first VEX ymm vector ops
- 32x8 i32 add,sub,mul
   - add I32_Naive bench/test builder to get better i32 mul coverage
   - minor refactoring all over

Change-Id: I13cc19ff37a2da0bcff289ba51baac08f456d6c5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/222485
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-20 18:20:00 +00:00
Mike Klein
d3e75a7a1c add(GP64 dst, int imm)
Change-Id: I6aaa1dc3ff3a9529f87ff58a2ae1fc5f59bc626b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221653
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-19 15:10:35 +00:00
Mike Klein
948045d1a5 make registers a little less verbose to work with
Change-Id: I92c6027e16af19112a5497854f1085715cc38e3d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221652
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-19 14:36:12 +00:00
Mike Klein
61703a643d first interesting instruction, sub(GP64,imm)
Change-Id: I89618ec826f54a24fce8ac5fa61ab27e8ec7d5c2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221651
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-19 14:19:12 +00:00
Mike Klein
056420428d Reland "extract Assembler so it can be tested"
This is a reland of a224fc1105

Changes since original:

  - switch fJIT_K to less error prone fJITMask
  - guard fJIT Assembler Program member with SKVM_JIT

Not really sure why the mips64el-Debug bot's compiler is crashing;
it does at least make sense to crash where it does... the file
includes SkOpts.h which includes SkVM.h.

If no reasonable code transformation can get it working again
I'll remove the bot.  The -Release version is fine, and mips64el
is one of those things I'd happily flush if it blocks progress.

In this end I think all this SKVM_JIT and Xbyak stuff should
go away and make things simple again, hopefully too simple to
crash GCC.  :|

Original change's description:
> extract Assembler so it can be tested
>
> And start documenting some structs we'll need
> to replace xbyak.
>
> Change-Id: I21c91642799a54e10af85afc8edbe12a9b4aa062
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221644
> Reviewed-by: Herb Derby <herb@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>

Cq-Include-Trybots: skia.primary:Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-SK_CPU_LIMIT_SSE2,Build-Debian9-GCC-mips64el-Debug
Change-Id: I6d7c27bc758b23c164ee67067cdfacc291e289fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221983
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-19 13:57:22 +00:00
Mike Klein
6b7c9d95f5 Revert "extract Assembler so it can be tested"
This reverts commit a224fc1105.

Reason for revert: breaking x86-64 bots without AVX2, e.g. Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-SK_CPU_LIMIT_SSE2

Original change's description:
> extract Assembler so it can be tested
> 
> And start documenting some structs we'll need
> to replace xbyak.
> 
> Change-Id: I21c91642799a54e10af85afc8edbe12a9b4aa062
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221644
> Reviewed-by: Herb Derby <herb@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>

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

Change-Id: Ie90d57f66e4d45f94db4ab4f485155533faddae1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221655
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-18 21:24:41 +00:00
Mike Klein
a224fc1105 extract Assembler so it can be tested
And start documenting some structs we'll need
to replace xbyak.

Change-Id: I21c91642799a54e10af85afc8edbe12a9b4aa062
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/221644
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-18 19:04:44 +00:00
Mike Klein
5640397c48 fix dst/arg aliasing issues
Any time we implement a Program::Instruction with multiple low-level
operations, we risk overwriting any arguments that alias the
destination.

This is why the _I32 tests are failing, mad_unorm8 where d == x.  We
want (x*y+x)/256+z, but end up calculating (x*y+x*y)/256+z when x == d.

We could fix this by never allowing any arguments to alias any
destinations, but most instructions don't have this problem, and doing
that blindly would bloat the register count significantly.

We could fix this by knowing which Ops may be prone to aliasing in any
backend, but I find that somewhat error prone and also a little
abstraction- level-violatey.  I would have thought, for instance, that
the mad_f32 Op might be vulnerable here, but it's actually not... in any
situation where there is aliasing, we actually lower it to a single
vfmadd instruction, never mul-then-add.

This sort of aliasing issue is going to keep coming back up again and
again, especially with 2-argument architectures like SSE.  Luckily it's
trivially easy to fix by reserving a single tmp register to use as the
result of all but the final instructions.

The interpreter is safe because all its switch cases are single r(d) =
... statements.  The right hand sides are evaluated before anything is
written back to a destination register slot.  Had it been written a
little differently, it could have easily had this same aliasing issue.

Change-Id: I996392ef6af48268238ecae4a97d3bf3b4fba002
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220600
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-13 16:53:29 +00:00
Mike Klein
3f593799da expand unit tests, fix extract
The mask-only special case for extract is wrong...
it never looked it its input!

This not only makes things correct-er, but oddly it also
makes them faster by breaking inter-loop data dependencies.

Disable tests for _I32... they're actually still broken
because of a much more systemic flaw in how I've evaluated
programs.  The _F32 and _I32_SWAR JIT code and all interpreted
code is just getting lucky.  o_O

While here, update the I32_SWAR code to use the same math as I32,
(x*y+x)/256 for unorm8 mul.  This just helps keep me sane.

Change-Id: I1acc09adb84c426fca4b2be5ca8c2d46d9678dd8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220577
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
2019-06-12 18:58:56 +00:00
Mike Klein
81756e4cae test and fix that we cover the right inputs
At head we're redoing any n<8 tail from the start,
not continuing from (n/8)*8 like we'd want.

Change-Id: I1a3d24cdffc843bbe6f3e01a163b6e3a20fdd0ca
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220556
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-12 18:44:58 +00:00
Mike Klein
22ea7e994b add Builder::dump()
I used to have a dump of the value program before it was
translated to registers, but it went away a while ago.
This restores it.

Change-Id: I9b8bfcb124843cad4b0dc44bdf0a03e95a0c83d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/219757
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2019-06-10 17:43:58 +00:00
Mike Klein
771633190e print SKVM test failures
Can sometimes be hard to know what's going on
on the bots without a little bit more debug help.

Change-Id: Ie556a8de88349170e9d9e44c16098223442838a2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218316
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-04 20:33:42 +00:00
Mike Klein
7b7077cc36 centralize test/bench SkVM builders
Eliminate the duplicate functionality,
and better testing for the bench builders.

Change-Id: If20e52107738903f854aec431416e573d7a7d640
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218041
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-04 16:55:59 +00:00
Mike Klein
267f50773c streamline SkVM test rebaselining
- keep expectations in resources/
  - overwrite automatically if needed
    so we can see the diff in Git

Change-Id: I2486b127ebcc7f40332fd0462e38b1af04d3e32b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/218038
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-03 22:24:50 +00:00
Mike Klein
081bf6667d best guess for -MSRTC bot
Change-Id: I5aa5f789180b9caba70952cc60a2e9bbcf3b5a97
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/217983
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2019-06-03 20:44:29 +00:00
Mike Klein
68c50d015b sketch an skvm
With all the thinking around a stack-based interpreter,
I figured I'd sketch out some ideas for a register VM too.

I kind of have the hunch that this is the direction that
will actually let us replace large amounts of Skia's CPU
backend with an efficient interpreter or JIT.

Change-Id: Ia2b5ba4a3fc27556f5b6ba95cd1ace46d3217403
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/216665
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-06-03 19:53:48 +00:00