Commit Graph

123 Commits

Author SHA1 Message Date
Brian Osman
ae87bf1e49 Remove SkSpan class template deduction guides
This effectively reverts a80ce1a36d,
and goes back to using SkMakeSpan (which works in C++14).

Change-Id: Iaa63c86b5acaadbdd60588b0a5c703820e810770
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/406938
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-05-11 20:06:29 +00:00
John Stiles
3738ef531b Move code generators to codegen/ directory.
We are up to having seven distinct types of codegen, and will soon have
an 8th (DSL C++).

Change-Id: I6758328390c234ba1d5c30c118199dbc820af52a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395817
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-04-13 15:34:24 +00:00
John Stiles
7134646ebd Migrate SkSLExternalFunction to the ir/ directory.
Change-Id: I8ca4fbba551624c61a4ebe7a4716750fe0b48196
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395818
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-13 14:17:16 +00:00
John Stiles
dbd4e6f0c0 Move ProgramKind and ProgramSettings types out of SkSL::Program.
This change will allow these types to be forward-declared; C++ doesn't
allow forward declaration of types declared inside a struct. Moving
these types out of Programs resulted in a large diff.

The Settings::Value helper class has been moved inside of the
IRGenerator. In practice, it was actually just an implementation detail
of how IRGenerator looks up caps-values by name. It seems very unlikely
that this will be necessary elsewhere going forward.

Change-Id: I6119417fae608f1c492a27de746d2b550ef8ca20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370836
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-16 19:30:48 +00:00
Brian Osman
21f570769e Fix complex lvalues in SkSL-to-SkVM
The old code only handled swizzles as the outermost expression(s) in an
lvalue. The new code removes that restriction, and puts all the logic in
writeStore, which is the only place it needs to exist.

The newly added test asserted before, and now passes.

Bug: skia:11178
Change-Id: I8083d9d478ad4dc993cb963d34a97c10965831b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358956
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-27 16:51:20 +00:00
Brian Osman
fa71ffaedb Reland "Fix field access and indexing of complex expressions"
Evaluating either kind of expression now works like all other
expressions - evaluate the inner part, then work with the resulting
values. Added unit tests for both of these that previously failed.

With this change, writeVariableExpression is only used for
VariableReference expressions, so adjust that, too.

Reland now safe, after fix to Value::operator[]

This reverts commit 1ea6d6051e.

Bug: skia:11178
Cq-Include-Trybots: luci.skia.skia.primary:Test-Debian10-GCC-GCE-CPU-AVX2-x86-Debug-All-Docker
Change-Id: I14782fcdfef33a47a46334447c5847976721b21f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359564
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-27 14:05:39 +00:00
Brian Osman
1ea6d6051e Revert "Fix field access and indexing of complex expressions"
This reverts commit f619079545.

Reason for revert: Some bots unhappy.

Original change's description:
> Fix field access and indexing of complex expressions
>
> Evaluating either kind of expression now works like all other
> expressions - evaluate the inner part, then work with the resulting
> values. Added unit tests for both of these that previously failed.
>
> With this change, writeVariableExpression is only used for
> VariableReference expressions, so adjust that, too.
>
> Bug: skia:11178
> Change-Id: Ia595be473b55f4bb03ec25897f9929835177257c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358529
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>

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

Change-Id: I56776139f9164b24b35a93307774e9b12c50054e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11178
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358959
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-25 22:03:11 +00:00
Brian Osman
f619079545 Fix field access and indexing of complex expressions
Evaluating either kind of expression now works like all other
expressions - evaluate the inner part, then work with the resulting
values. Added unit tests for both of these that previously failed.

With this change, writeVariableExpression is only used for
VariableReference expressions, so adjust that, too.

Bug: skia:11178
Change-Id: Ia595be473b55f4bb03ec25897f9929835177257c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358529
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-25 20:52:19 +00:00
Brian Osman
ce75036b3e SkSL: Remove ByteCode interpreter, generator, etc.
Change-Id: Ib150e6d6d3de34a85ce8051eea843ab3b2d7ab75
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356921
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-22 14:38:41 +00:00
Brian Osman
dd50b0c406 Add ExternalFunction support to SkSL-to-SkVM
Change-Id: I81c9a6ddcd65d40ebced18c7cfb1cad51066ba7b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/356297
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-20 18:03:26 +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
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
Brian Osman
035606d291 Enable matrix-negation interpreter test
This was disabled a long time ago, when this was (incorrectly) forbidden
by the IR generator, most likely.

Change-Id: I14585d249104b263c152fa59cbeba0c4e9a2e074
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354666
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-01-15 21:30:46 +00:00
John Stiles
af9b58e287 Fix fragment processors and unit tests which mix ints and floats.
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: Iff5e2820806b9419afdfcbf25d4a7f96f2eeeccb
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353416
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-14 00:24:45 +00:00
Brian Osman
9333c8750d Support ES2-compliant for loops in SkSL-to-SkVM
Such loops must be unrollable, so that's what we do.

Bug: skia:11094
Change-Id: I1b34917b6f2d015ae7867415d0120a5df0ffd618
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353619
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-13 22:39:45 +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
Brian Osman
c92df39de0 Convert interpreter "compound types" test to SkVM
This is the first test that used uniform data, so fix up how uniforms
work in the generic SkSL-to-SkVM function.

Bug: skia:11094
Bug: skia:11096
Change-Id: Ie391c1a6b8b68f0f4f014d7e767d7b5101341fab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352739
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-12 13:58:46 +00:00
Brian Osman
c39a143632 Remove ByteCode from some failure-expecting interpreter tests
Bug: skia:11127
Change-Id: I3a7fda8bb62f9cf9b6c83441703f537e75461d07
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352509
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-11 20:42:35 +00:00
John Stiles
54e7c056d9 Separate out built-in SkSL types into their own struct.
Previously, these were in SkSL::Context directly. This change doesn't
remove them from the context entirely, but it gives them a dedicated
subclass and firewalls them off from the rest of the context.

Change-Id: I0c344bf7436a11b8494a5fe7542d0a4ef1ece964
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352502
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-11 20:02:24 +00:00
Brian Osman
11233d2f12 Convert InterpreterFunctions test to SkVM
Change-Id: I20a40af290fd907e1b1a57a2fd09659d62db3af3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352508
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-11 19:56:24 +00:00
Brian Osman
e2416d7697 Convert early-return interpreter test to SkVM
This feature works fully on the skvm backend, and the byte code backend
is going to be removed soon.

Bug: skia:10852
Change-Id: I4711fcea7c85232c0b740f3b3c012f47310a768e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352258
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-11 15:21:26 +00:00
Brian Osman
54515b7b2b Support function calls in SkSL-to-SkVM
Bug: skia:10680
Change-Id: I8697bdc157d250f3c390c7f49074318aa8c7bdab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351918
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-09 00:47:48 +00:00
Mike Klein
606488ad68 back to exactCompare, fixed
Change-Id: Ifd7883a4b327aae9fc0a984f08755d6d6f57f72e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351018
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2021-01-07 21:25:31 +00:00
Brian Osman
be0b3b7363 Strip down SkSL::ExternalValues, limit them to functions
Previously ExternalValues were flexible, and could be used as raw values
(with the ability to chain access via dot notation), or they could be
callable. The only non-test use-case has been for functions (in
particles) for a long time. With the push towards SkVM, limiting
ourselves to this interface simplifies things: external functions are
basically custom intrinsics (and with the SkVM backend, they'll just get
access to the builder, and be able to do any math, as well as
loads/stores, etc).

By narrowing the feature set, we can rename everything to reflect that,
and it's overall clearer (the SkSL types now mirror FunctionReference
and FunctionCall directly, particularly in how they're handled by the
CFG and inliner).

Change-Id: Ib5dd34158ff85aae6c297408a92ace5485a08190
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350704
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-01-07 14:28:41 +00:00
Brian Osman
f4a77739f3 Use SkVM for (many) interpreter tests
Continue to test everything with the ByteCode interpreter, and run most
tests with the new SkSL-to-SkVM utilities, as well. A few tests rely on
features that aren't yet implemented (function calls, looping), and some
of the bespoke tests (that don't use the test() helpers) use even more
exotic features that need to be implemented or disallowed in the IR
generator. This is getting us closer to not needing ByteCode at all,
though.

Refactored a bunch of the helper code to reduce copy-paste among the
many different 'test' functions.

Change-Id: I138d4a24266f2d862742245c5ee895d86c01018e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350560
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-06 21:58:40 +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
Brian Osman
818fd6d357 Disallow while and do-while loops in runtime effects
Bug: skia:11095
Bug: skia:11127
Change-Id: I1537490d59632f63c165a38a1dbfc79ddfa9c380
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349056
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-12-30 22:17:19 +00:00
Brian Osman
20fad32064 SkSL: Hide most integral types from runtime effects
'int' is the only integral type that exists in GLSL ES 1.0 (and it's not
really guaranteed to be an integer). This enforces the same restriction
on runtime effects - no unsigned integers, and no short or byte types.

Bug: skia:11093
Change-Id: I938f1e0e125dc8347507f428b46b51c66033c752
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347046
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-12-23 18:28:17 +00:00
Brian Osman
c0f2b64342 SkSL: Hide non-square matrices from runtime effects
These don't exist in our minimum spec (GLSL ES 1.0)

Bug: skia:11093
Change-Id: Ia2d871199fff2a98dcd517c1eebe46decb0c2dfb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346657
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-12-22 19:31:42 +00:00
Brian Osman
d6f2338ab1 SkSL IR normalization: Convert while loops to for loops
Bug: skia:11095
Change-Id: Icd69df40675e5ecde5004e04a7dcd78eedf8343c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344765
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2020-12-16 23:03:23 +00:00
Brian Osman
ff44584b47 SkSL: Disallow '%' and '%=' on non-integral types
Bug: skia:11072
Change-Id: Ic24e40bfea5bf1d2d14c0f681632228a5ecc7104
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342929
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
2020-12-10 22:19:49 +00:00
John Stiles
1b27c3d7a3 Check array bounds when a constant array index is used.
This sort of error would be detected by most backend compilers. This
case was also detected by the bytecode generator. It's easy for us to do
a similar check during SkSL IR generation and report the error sooner.

Also, `convertIndex` had migrated a few hundred lines away from
`convertIndexExpression`, so I moved it back to live next to its parent.

Change-Id: I715d3abf42581782b55ba60df30d0296355667d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341377
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-12-07 18:17:30 +00:00
John Stiles
076e9a2f34 Disallow returning array types in SkSL.
This is illegal in older versions of GLSL and in Metal. We now fail at
SkSL compilation time and properly report the error.

Change-Id: I6ddaeabff5386a1ed6ca3eb8703a6035476ec77a
Bug: skia:11021
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339298
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-12-03 16:27:39 +00:00
Brian Osman
0006ad01ce Stop cloning builtin functions
Previously, any builtin functions would be optimized as a side-effect of
optimizing programs that used them. Now that shared elements aren't
being optimized in that way, we explicitly optimize any shared modules
when they are first created. We don't remove dead elements, but we
we do substitute settings, simplify, and inline.

Bug: skia:10905
Change-Id: I701b5e9f52fb880ef3e6f4c67694d08602f47e95
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336440
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-20 15:02:54 +00:00
Brian Osman
1f71f433ff Always enable SkSL's ByteCodeGenerator, disable interpreter in Google3
The ByteCodeGenerator is needed for SkSL-via-skvm, but almost no one
needs the ByteCode interpreter.

Bug: b/172773885
Change-Id: Ia7b6768dbc00c6c78b971ba50f0b702536bbd5b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336016
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-11-18 21:30:45 +00:00
Brian Osman
d7e7659cad Move GrShaderCaps from Program::Settings to Compiler
This ties the caps to the compiler instance, paving the way for
pre-optimizing the shared code. Most of the time, the compiler is
created and owned the GPU instance, so this is fine. For runtime
effects, we now use the shared (device-agnostic) compiler instance
for the first compile, even on GPU. It's configured with caps that
apply no workarounds. We pass the user's SkSL to the backend as
cleanly as possible, and then apply any workarounds once it's part
of the full program.

Bug: skia:10905
Bug: skia:10868
Change-Id: Ifcf8d7ebda5d43ad8e180f06700a261811da83de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331493
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-04 19:38:33 +00:00
Mike Klein
51dc28505f allow early returns
For posterity, here's my initial, wrong thinking:

    If we squint at "return foo" and read it as "result = foo; goto
    end_of_program", and then remember we can always skip forward jumps (and
    where's further forward than end of program?), early returns turn out to
    work just like a store.

The reason this is wrong is that by the time we reach a final return,
the entire mask stack has been popped back down to its original default
ffffffff (active) state.  But that return shouldn't override any prior
returns.  So that scheme isn't quite right.

Instead we accumulate the result by disabling updates to lanes that have
already returned.  By the time we're done, all lanes should have hit
_some_ active return, now asserted.

Bug: skia:10852
Change-Id: I27b05f04a60ff4a5f2fe5f59bf398c3f7224a41b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/327457
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-10-20 14:33:31 +00:00
Brian Osman
401a366eb1 Fix swizzle-of-swizzle lvalues in ByteCodeGenerator
Bug: skia:10785
Change-Id: I01708af63d7e2ffc160022074ea9ff2b3c69eab5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320638
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-09-29 20:34:58 +00:00
Ethan Nicholas
7bd6043029 moved SkSL Block's data into IRNode
Change-Id: Id1066c7c2aeffce47fcd6823cb01a58b52e450c2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319676
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-09-25 19:33:19 +00:00
Brian Osman
32d53550c8 Remove unsafe compiler methods related to external values
We don't want to be polluting the global namespace with external values,
especially when the typical/recommended way to use the Compiler is with
a single long-lived instance. Force client code to manage ownership (the
only non-unit-test case was already doing this), and pass external
values to convertProgram, so they can be added to the Program's symbol
table.

Change-Id: If4c1db5e48a62e2cf4333b8d80420f2dfede27ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319125
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-09-24 14:12:08 +00:00
John Stiles
759880ab05 Pass non-temporary Program::Settings to convertProgram.
This will fix ASAN errors such as these:
https://logs.chromium.org/logs/skia/4e924a3651708511/+/steps/symbolized_dm/0/stdout

Change-Id: I49756df6e077cc8813f51f70082c88d861fa337f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316451
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2020-09-11 16:37:34 +00:00
John Stiles
7571f9e490 Replace 'typedef xxxxx INHERITED' with 'using INHERITED = xxxx;'.
Mechanically updated via Xcode "Replace Regular Expression":

  typedef (.*) INHERITED;
    -->
  using INHERITED = $1;

The ClangTidy approach generated an even larger CL which would have
required a significant amount of hand-tweaking to be usable.

Change-Id: I671dc9d9efdf6d60151325c8d4d13fad7e10a15b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314999
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-09-03 03:41:26 +00:00
John Stiles
534d79979d Fix constness of ExternalValue* inside ExternalValueReference.
Objects in the symbol table are intentionally constant. However, when
converting "ExternalValue" AST nodes into symbols via symbol table
lookup, IRGenerator::convertIdentifier was casting away constness
because ExternalValueReference held a non-const pointer. Fixing this
involved significant ripple-effect additions of "const" throughout the
ExternalValue class and its subclasses.

These changes generally appear to be benign, but one interesting edge
case is `ExternalValue::write`, which intuitively does not seem to make
sense as a const method. However, invoking `write` should not alter the
ExternalValue object itself; rather, it is intended to alter the
*external value* that is being referenced. (In practice, nothing invokes
write() anyway except for one unit test, which continues to pass.)

This issue was discovered while converting casts to `as<T>()` calls.

Change-Id: I8ff6a477e475833d2a99c72f1c79c766b57767ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311276
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2020-08-18 13:44:05 +00:00
John Stiles
c1c3c6d70d Enable ClangTidy flag bugprone-suspicious-string-compare.
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-string-compare.html

Find suspicious usage of runtime string comparison functions.
This check is valid in C and C++.

Checks for calls with implicit comparator and proposed to
explicitly add it:

  if (strcmp(...))       // Implicitly compare to zero
  if (!strcmp(...))      // Won't warn
  if (strcmp(...) != 0)  // Won't warn

Checks that compare function results (i,e, strcmp) are compared to valid
constant. The resulting value is

  <  0    when lower than,
  >  0    when greater than,
  == 0    when equals.

A common mistake is to compare the result to 1 or -1:

  if (strcmp(...) == -1)  // Incorrect usage of the returned value.

Additionally, the check warns if the results value is implicitly cast
to a suspicious non-integer type. It’s happening when the returned
value is used in a wrong context:

  if (strcmp(...) < 0.)  // Incorrect usage of the returned value.

Change-Id: I001b88d06cc4f3eb5846103885be675f9b78e126
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310761
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2020-08-16 03:54:08 +00:00
Ethan Nicholas
e8ad02c6e9 Revert "Reland "Revert "Omit dead SkSL functions"""
This reverts commit a15f2bfb6f.

TBR=bsalomon@google.com, brianosman@google.com

Change-Id: I64cc7830281ee69bf3377c5b6daa3d96ccf4769e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293976
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-04 02:46:55 +00:00
Ethan Nicholas
a15f2bfb6f Reland "Revert "Omit dead SkSL functions""
This reverts commit fd1173ac71.

Reason for revert: TSAN failure: https://chromium-swarm.appspot.com/task?id=4c93823229fa6a10

Original change's description:
> Revert "Revert "Omit dead SkSL functions""
> 
> This reverts commit 7c969f26bc.
> 
> Change-Id: I8fb99f271e2ecaeb83d570cc2d2cf8851bc776f0
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293338
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

TBR=bsalomon@google.com,brianosman@google.com,ethannicholas@google.com

# Not skipping CQ checks because this is a reland.

Change-Id: Iab3ee5a336074676ebd0b761922ececf189752ef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293843
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-03 17:14:18 +00:00
Ethan Nicholas
fd1173ac71 Revert "Revert "Omit dead SkSL functions""
This reverts commit 7c969f26bc.

Change-Id: I8fb99f271e2ecaeb83d570cc2d2cf8851bc776f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293338
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-03 14:22:18 +00:00
Ethan Nicholas
7c969f26bc Revert "Omit dead SkSL functions"
This reverts commit 97fe0cbed2.

Reason for revert: ASAN failures

Original change's description:
> Omit dead SkSL functions
> 
> Now that SkSL inlines functions, dead functions are very common. This
> change causes them to be omitted from the final output.
> 
> Change-Id: Ie466a3f748812eff1a368498365c89d73ab0b7be
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292684
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

TBR=bsalomon@google.com,brianosman@google.com,ethannicholas@google.com

Change-Id: Id20c5be67dd574d30d6f978ba610e43aa5018416
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293241
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2020-06-01 15:00:00 +00:00