Commit Graph

292 Commits

Author SHA1 Message Date
John Stiles
26fdcbb38b Implement constant folding for (bool == bool) and (bool != bool).
We already had support for &&, ||, ^^ but somehow the common cases of
== and != were not implemented in the constant-folder.

This CL also updates the test to return a green/red color on success or
failure, instead of assigning arbitrary numbers into sk_FragColor that
don't mean anything. The long-term plan is to signal success or failure
of each test by color code; we can display these colors as swatches in a
GM slide for testing purposes.

Change-Id: I0810108b3c6b656a60cd8aa64ceefd765eff0157
Bug: skia:11112
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355984
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-20 14:22:35 +00:00
Brian Osman
b8ebe237c3 Reland "Support indexing by loop variables in SkVMGenerator"
This reverts commit b7e836cee9.

Change-Id: I3c39a928ba4a9a2863b616f2a500975294b03860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355980
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-19 22:33:46 +00:00
Mike Klein
b7e836cee9 Revert "Support indexing by loop variables in SkVMGenerator"
This reverts commit ebf569004f.

Reason for revert: std::clamp is c++17

Original change's description:
> Support indexing by loop variables in SkVMGenerator
>
> Bug: skia:11096
> Change-Id: I25a91bacf1c3455ac67422fb0e59b9b152c2054a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354667
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>

TBR=mtklein@google.com,brianosman@google.com,johnstiles@google.com

Change-Id: I0590cf7fe626fb59be3381b5e8eb66a9a2a9e8cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11096
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356056
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2021-01-19 21:27:58 +00:00
Brian Osman
ebf569004f Support indexing by loop variables in SkVMGenerator
Bug: skia:11096
Change-Id: I25a91bacf1c3455ac67422fb0e59b9b152c2054a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354667
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-19 20:49:15 +00:00
John Stiles
f8dfc3b518 Generate valid Metal code when globals reference one another.
`globalStruct` is now named `_skGlobals` and is passed around directly
by reference, with no additional helper variable (`_globals`) at all.

Change-Id: Icc5566d2212afd14a4d43700e89f50bedcc8b45f
Bug: skia:11168
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355717
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-19 17:07:24 +00:00
John Stiles
9d7aa41081 Reorder GLSL output so that functions are emitted last.
The Inliner likes to move function bodies around; after inlining, code
can inadvertently move upwards, above ProgramElements that the code
relies on. We work around this by always emitting functions last.

Change-Id: Ie5486cc3a79a478920342fb9f578d575486fb4cf
Bug: skia:11186
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354669
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-15 23:15:46 +00:00
Brian Osman
ea485e5285 Enforce ES2 limits on indexing expressions (in runtime effects)
This enforces an even stricter version of the rules from GLSL ES 1.0
Appendix A, Section 5. Essentially, indices (to arrays, vectors,
matrices) must be made of literals, loop indices, and expressions made
of those two.

Bug: skia:10837
Bug: skia:11096
Change-Id: I437a5ed64da58e24d5991ddbde68859f5214e98b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354665
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-01-15 22:49:27 +00:00
John Stiles
4f2bcff08e Implement Type cloning for enums and structs.
As far as I know, there shouldn't be a way to introduce a struct or enum
other than at global scope; the keywords are not accepted inside a
function body. In fact, I wasn't able to find a way to exercise these
code paths in practice. But we now have concrete assurance that any
possible type can be cloned into a symbol table safely; all Types are
either built-in (available everywhere by design) or are clonable.

Change-Id: I4b006b6cab995b3e598b683736ab9689828629c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354664
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-15 21:45:56 +00:00
John Stiles
6a1a98c313 Fix for fuzzer-discovered use-after-free.
The inliner discovered that when a binary expression is inlined, its
type is not cloned into the destination's SymbolTable. This meant that
when the inlined-from function was later dead-stripped, the type pointer
would become dangling. Did a quick pass over inlineExpression and
inlineStatement and ensured that types are always copied.

Also found that `copy_if_needed` was making a copy of eligible types
each time one was encountered, instead of making one copy and reusing
it. This is fixed as well.

Change-Id: Iee3259ab038dfb04034bf0110af1909ccffec3de
Bug: oss-fuzz:29444
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354219
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-01-15 15:11:00 +00:00
Brian Osman
4cf85073e6 Enforce (valid) array sizes in many more places
Unsized arrays are now allowed in exactly one place: On the declaration
of an interface block. This satisfies the one existing use-case, which
is the gl_in (sk_in) declaration for geometry shaders. There is no other
useful scenario, and most of our backends don't support them anyway.

Several spots were using less strict checks when attaching sizes to
arrays, allowing for zero or negative-sized arrays, so those are all
fixed now.

The existing tests that initialize arrays are still a problem, because
Metal doesn't support that (neither does GLES2). Also, ArrayConstructors
has gone from generating an error in the Vulkan backend, to invalid
SPIR-V.

Bug: skia:11013
Bug: skia:11127
Change-Id: Ib08dfe9aeec96bf605661665d6f166419d27e8bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353817
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-01-14 22:14:59 +00:00
Mike Klein
a738fb5d58 restore select() in bool->int
Change-Id: Ia3ac338bef376aa1649569b9ebd3f7feb23ffd52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353936
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2021-01-14 21:09:58 +00:00
John Stiles
774f45bc07 Enforce strict type coercion between int and uint.
This better matches GLSL's type coercion behavior.

Change-Id: I73fcfd8a9e57fd4cdb1692074d73ebd8fb788ac2
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353712
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-14 19:38:55 +00:00
John Stiles
ff4f76352d Update int/float mismatch test to include uints.
Change-Id: I47d02ca63ce64d9cfb3de0888d84b2b8a822f2b5
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353710
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-14 19:38:30 +00:00
John Stiles
4a2d651243 Enforce stricter type coercion rules in SkSL.
Literals are still flexible; we still allow `1` to coerce to float.
However, we no longer accept code like `int x = sqrt(2);` or
`int x = 0; float y = x;` without an explicit cast.

Change-Id: Ieb294a4877447e2336252f876e8bc489d1e4a59a
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353417
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-14 18:59:45 +00:00
John Stiles
a60fb178b5 Add SPIR-V support for casts to boolean scalar.
Change-Id: Icd8982d604881effee31cc1392e2717cb112d06d
Bug: skia:11172
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353629
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-14 18:35:25 +00:00
Brian Osman
890b2b406a Disallow switch statements in runtime effects
Bug: skia:10680
Change-Id: Ic77f7355866363ef476a93d8da180cf53207fa6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353707
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-01-14 17:45:16 +00:00
Brian Osman
01f322cce4 Move all runtime effect error tests to runtime_errors
Also renamed Discard to IllegalStatements, and added testing of while
and do loops.

Change-Id: Ibacf69131267a0436808e2e022ad126704af16ef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353706
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-01-14 17:40:54 +00:00
Mike Klein
aebcf73404 always init function fReturnValue
It's not sound to pass undefined (skvm::NA) values into select(),
but this is working today because the F32a argument is 'fixing' it.

The first time through this snippet updating fReturn value,

    int i = 0;
    for (skvm::Val& slot : currentFunction().fReturnValue) {
        slot = select(returnsHere, f32(val[i]), f32(slot)).id;
        i++;
    }

the call to f32(slot) creates an F32{builder, NA}.  We pass that to
select() and that argument's F32a(F32) constructor, resulting in
F32a{builder, NA, 0.0f}.  Then when we need that as an F32, we resolve
it as splat(0.0f) because the F32a's id field is NA.

In short, best to remove F32a. :)

Added some SkASSERTs that would have caught this.

Change-Id: I67324cf20ad39ca555e69b9c407f379d14046043
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353838
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-14 17:22:04 +00:00
Mike Klein
a517cfc3f4 slightly tweak bool->int
I'm looking to phase out I32a/F32a, and rewriting this expression to
avoid select() makes it easier, making these types unused except in
SkVM.{h,cpp}.

There's no particular reason beyond making that refactor easier to do
this: SkVM can convert select(cond, splat(1), splat(0)) into cond & 1
itself, and once I'm done with removing I32a/F32a, if we prefer select
we should be able to rewrite this back as

    dst[i] = skvm::select(i32(src[i]), 1, 0);

Change-Id: I562a112e54fdc2578802db02f6754c64a12798cc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353837
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-14 17:14:33 +00:00
John Stiles
e7e68efd9f Add unit test for int/float mismatch error detection.
The "disallowed" tests are largely allowed in the current code, but all
fail properly in the followup CL.

Change-Id: I8e03570165480b60db9701ac1a782e1124ded56b
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353617
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-14 16:56:53 +00:00
John Stiles
48c2884c70 Add SPIR-V support for boolean vector typecasts.
Previously, `writeVectorConstructor` did not consider boolean types at
all when converting scalars to a different type. Now, this code reuses
the existing logic from `castScalarTo(Float|SignedInt|UnsignedInt)`
which supports Booleans. Added `castScalarToBoolean` to cover going in
the opposite direction.

Change-Id: I5479ab181b9b721db7fbff0bdc01718ce8f9f9b9
Bug: skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353625
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-14 16:50:03 +00:00
John Stiles
e318355a67 Add unit test for scalar conversion constructors.
This revealed a gap in our SPIR-V scalar constructor support;
typecasting a number to bool would lead to an ABORT.

Change-Id: Idac6d7ba34adfd214ed3cad8139e22d7170456f0
Bug: skia:11172
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353628
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-14 15:53:03 +00:00
Brian Osman
3d81fdcbd2 Add unit tests for for-loop unrolling
Change-Id: I350a6768ac124362b0d3e0f17e7a026265acf804
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353627
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-14 14:38:16 +00:00
John Stiles
d9d5271b6b Factor out SPIR-V typecasting helper functions.
The test diffs look scary, but the only actual change is a minor
renumbering of IDs. The actual logic is the same.

Change-Id: I5ecc26c8581a4c01834932ff0291deba7d9e4618
Bug: skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353622
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-14 00:00:15 +00:00
Brian Osman
77ba8103d3 In runtime effects, verify that loops conform to ES2 rules
Bug: skia:11094
Change-Id: I68a08e79d29579901b74daca3c22f5112fbb3c8c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353356
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-13 21:49:44 +00:00
John Stiles
a65441b3c4 Update tests which mix int and float types without casts.
These need to change because type coercion in SkSL is about to become
more strict in a followup CL; we are disallowing expressions that mix
ints and floats without a cast.

Change-Id: I0f6c3cba53fb67078f447345338262c153236c51
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353102
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-13 19:47:13 +00:00
John Stiles
d5e59b6024 Allow type-fluid GLSL-style vec2(int, bool) ctors in SkSL.
Note that GLSL accepts these sorts of constructors natively, but Metal
and SPIR-V do not. In the generated IR we actually add a cast for
subexpressions where the type does not match. These casts can be seen in
the final output for both GLSL (where they are no-ops) and Metal/SPIR-V
(where they are essential).

This change exposed some missing SPIR-V functionality (vector casts do
not support bool types). This can be fixed up in a followup CL; these
casts were previously disallowed by SkSL entirely, so there won't be any
of them in existing code.

Change-Id: I54ae922e91b38bed032537496428747a081dc774
Bug: skia:11164, skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353576
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-13 19:45:14 +00:00
John Stiles
3c1917983b Update VectorConstructor unit test with mixed-type ctors.
GLSL allows mixed types inside a vector constructor, but SkSL currently
doesn't handle it well; some cases don't compile, and others generate
bad code. This will be fixed in a followup CL.

Change-Id: Ia98b498f320b8fa91595404730f6cdc836615140
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353577
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-13 19:09:16 +00:00
Ethan Nicholas
67a0a8a787 Fixed error reporting on invalid SkSL assignments
Change-Id: I9859081a14b110731f943e09fdd94dc10e0c9dfc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353580
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-01-13 18:34:44 +00:00
John Stiles
059bea1380 Simplify IRGenerator::coerce.
I started unpacking the mechanics of type coercion, and realized that
the second half of the function was looking up the Symbol for a Type
based on its name (Types are already Symbols), converting that Symbol
back into a Type (we started with a Type anyway), wrapping that Type
in a TypeReference, then calling that TypeReference (which always
calls convertConstructor).

This CL cuts out the middle steps and simply calls convertConstructor
directly. A test was added to confirm that an earlier error encountered
on the CQ is no longer occurring.

Change-Id: I76aae455a301afe4e67ef989d9dfe11f47ed36ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353105
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-01-13 15:43:44 +00:00
John Stiles
4b7fdc493e Construct IntLiterals with type fIntLiteral.
Change-Id: I96a6ef80849f93bba593116fd482f69ff787de5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353039
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-01-12 19:14:21 +00:00
John Stiles
508eba7578 Implement constant folding for vector*scalar ops.
Change-Id: I96b547de4fe4b73096fb26d0ef21a4e7555ca06a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352238
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-11 21:03:55 +00:00
John Stiles
74192fde0d Migrate constant folding tests into a separate directory.
This CL also adds tests for vector*scalar and scalar*vector folding.
We currently do not constant-fold these, but support will be added in a
followup CL.

Change-Id: I68d7374ae15ab2f4d805a095803b645c92fb03d9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352237
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-11 18:05:18 +00:00
John Stiles
b270c7e5e1 Remove rewrite of true ^^ expr to !expr.
This optimization doesn't perceptibly improve the generated code; it
just replaces a binary expression with an equivalent unary one.

Change-Id: Ib6cd2732a22c26978665c57ee00d7b5e5d0a0aee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352123
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-11 15:53:30 +00:00
Mike Klein
00e43df25b rename Arg to Ptr
This makes almost all existing code read more clearly.

Change-Id: I314331d9aa2ecb4664557efc7972e1a820afc250
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352085
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2021-01-08 20:50:15 +00:00
John Stiles
d2f51b1806 Fix fuzzer-discovered optimizer crash.
The CFG/definition map are no longer valid after replacing an expression
entirely. Swizzle-of-swizzle optimization was another case where the
optimizer would replace an expression wholesale, but failed to set the
needs-rescan flag.

Change-Id: Ida0363d738cd1d3ac2a48c824aa04065a7ca16b7
Bug: oss-fuzz:29085
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351776
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-01-08 13:03:02 +00:00
John Stiles
7cbb09c2fe Report a parsing error when invalid tokens are detected.
Change-Id: I75f907ca673ee67f5d623b032128b97833070a0b
Bug: skia:10931
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351504
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-07 21:49:51 +00:00
John Stiles
2787bc854f Add unit test for invalid tokens in input stream.
Change-Id: If6b23d03b02028b51f96e97080cbd7d34cc33b8f
Bug: skia:10931
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351503
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-07 20:50:01 +00:00
John Stiles
bd058477e2 Constant-propagate the ! prefix onto constant boolean expressions.
This will flatten out expressions such as `!false` or `!true`. We
already had a similar fix-up at IR generation time which handled simple
cases, but this will catch more complicated ones like `!sk_Caps.xxxxx`
(since caps bits are only flattened out at constant propagation time).

Change-Id: I04282809d9a784266a64dbcafd097f3b0662806c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351497
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-07 20:09:49 +00:00
John Stiles
df069c9ec7 Remove compile-time constant support from PrefixExpression.
This is not actually necessary now that constantPropagate can fully
flatten out unary negation into its constant operands. The compilation
results don't change at all.

Change-Id: I7ab55bd3720413609d799dd866e1703973cb2626
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351202
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-07 18:14:22 +00:00
John Stiles
9cfaa4ffa1 Flatten nested vector constructors when emitting SPIR-V.
This fixes SPIR-V code generation when encountering nested constructors
like `float3 v4 = float3(float2(1), 1.0);` as featured in our unit test
VectorConstructors.sksl.

Change-Id: I3a0c4b466b3cb17ba50bd264f899e59c55c768ed
Bug: skia:11141
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350032
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-01-07 15:47:11 +00:00
John Stiles
cd80689192 Deduplicate vector constants in SPIR-V output.
Many of our shaders generate the same vector constant dozens of times,
e.g. Gaussian blur uses float4(1) repeatedly. This change avoids
re-emitting redundant vector constants.

Change-Id: I22a71cd8b2783fb997f52d485b49031f64ca6d96
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350701
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-06 21:05:50 +00:00
John Stiles
acb091f71d Avoid emitting duplicate constant values in SPIR-V.
Previously, we had constant-value deduplication, based on the SkSL type
of the constant. However, we were still generating redundant constants,
because we would emit a separate constant for Float(n) and Half(n), or
Int(n) and Short(n), even though we generate the exact same instruction
for these constants. We now deduplicate based on the type's number-kind,
separating constant literals into three categories: floats, signed ints,
and unsigned ints. This better matches our type-handling in
getActualType.

Change-Id: I5777d4b3d567839b7aa72dc8de76908c18fc387e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350031
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-06 18:27:10 +00:00
John Stiles
32d68537a8 Add SkVM support for conversion constructors to and from boolean.
Change-Id: Ia4a1c38161046b94dc56a1a76704766f1e14aab7
Bug: skia:11131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350019
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-06 15:23:09 +00:00
John Stiles
53f0ddfa4e Unify conversion constructor simplification code.
Previously, the IR generator had code which could simplify conversion
constructors like `int(1.23)`. Separately, the optimizer's constant
propagation pass had its own separate implementation of these
simplifications as well.

This CL unifies the two implementations. Previously, the constant-
propagation pass version of the code only supported integer literals, so
this change also improves our code generation slightly.

Change-Id: I32c70a5f2aed210d03bef3166b1178a2d40cdabd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350024
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-06 15:13:29 +00:00
Brian Osman
cdde253e40 Runtime effects: Disallow bitwise ops and integer remainder
Bug: skia:10680
Bug: skia:11088
Bug: skia:11127
Change-Id: I25ea288d03df13147b31bc4ca4b224bbe2fa924e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350030
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-06 14:05:59 +00:00
John Stiles
ba4b0e93e3 Add support for number(boolean) and boolean(number) casts in SkSL.
Previously `number(boolean)` casts were converted to a ternary during
IR generation, and `boolean(number)` casts caused an error.

Metal and GLSL should support this cast as written. SPIR-V needed a
little bit of logic to handle converting the boolean to a number via
OpSelect.

Change-Id: I0069781e2b5a26a25c8625ab41c2392342bfd10d
Bug: skia:11131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349066
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-05 21:19:45 +00:00
Mike Klein
f471c827fb Stop calling schedule()
The new unit test demonstrates load/store reordering is error-prone.

At head we're allowing loads from a given pointer to reorder later than
a store to that same pointer, and boy, that's just not sound.  In the
scenario constructed by the test we reorder this swap,

   x = load32 X
   y = load32 Y
   store32 X y
   store32 Y x

using schedule() (following Op argument data dependencies) into

   y = load32 Y
   store32 X y
   x = load32 X
   store32 Y x

which moves `x = load32 X` illegally past `store X y`.
We write `y` twice instead of swapping `x` and `y`.

It's not impossible to implement that extra reordering constraint: I
think it's easiest to think about by adding implicit use edges in
schedule() from stores to prior loads of the same pointer.  But that'd
be a little complicated to implement, and doesn't handle aliasing at
all, so I decided to ponder on other approaches that handle a wider
range of programs or would have a simpler implementation to reason
about.  I ended up walking through this rough chain of ideas:

    0) reorder using only Op argument data dependencies          (HEAD)
    1) don't let load(ptr) pass store(ptr)                      (above)
    2) don't let any load pass any store              (allows aliasing)
    3) don't reorder any Op that touches memory
    4) don't reorder any Op, period.

This CL is 4).  It's certainly the easiest and cheapest implementation.
It's not clear to me that we need this scheduling, and should we find we
really want it I'll come back and work back through the list until we
find something that meets our needs.

(Hoisting of uniforms is unaffected here.)

Change-Id: I7765b1d16202e0645b11295f7e30c5e09f2b7339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350256
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2021-01-05 20:29:46 +00:00
John Stiles
61b2e81c4f Fix type error with Metal mod(vec, float) intrinsic.
The code didn't take into account that x and y might be different types.
(This bug was not actually harmful; type coercion allowed the code to
compile even with the wrong type. The float would be silently splatted
into a vec and the rest of the code would work as-is.)

Change-Id: Ib76bc733f76304e451ef9197421b4bc22e29e49c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348888
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-05 18:07:04 +00:00
John Stiles
ab8ed92a0c Add unit test for casting between float, int and bool.
This actually exposed a latent bug: we don't support bool(1.23) or
bool(1) casts, but these are valid in GLSL:

https://www.khronos.org/opengl/wiki/Data_Type_(GLSL)#Conversion_constructors

"to bool: A value equal to 0 or 0.0 becomes false; anything else is
true."

Change-Id: Ia929a09914ffc96f081d0402d7bb05b5428f8db6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349977
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-05 16:30:11 +00:00