Commit Graph

63 Commits

Author SHA1 Message Date
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