Commit Graph

368 Commits

Author SHA1 Message Date
Brian Osman
552fcb9a1b Remove flexible runtime effects entirely
All internal usage has migrated to MakeFor..., this removes the old
program kind, and updates some tests.

Bug: skia:11813
Change-Id: I56733b071270e1ae3fab5d851e23acf6c02e3361
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402536
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-04-29 16:02:27 +00:00
John Stiles
fb7d378a1a Add test demonstrating swizzled constant folding.
At present, this is a missed optimization opportunity. These will be
optimized in a followup CL.

Change-Id: I8882058900cdc12c8ab0df03e36ebfb9d8022f01
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402580
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>
2021-04-29 15:59:37 +00:00
John Stiles
958a9395e6 Fix array-of-struct name mangling in Pipeline stage.
There aren't many cases where array-of-struct types are useful in
ES2, but it looks like function parameters are one such case:
http://screen/7Fnc7GhewAkUK3j

Change-Id: I23410a3824a3c202c12147d6939586cc0e55a9ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402397
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-04-28 21:02:36 +00:00
John Stiles
65d7ab2c07 Loosen ES3 restrictions in Runtime Effects for debugging.
This CL adds a RuntimeEffect option flag which skips over the various
`strictES2Mode` checks sprinkled throughout IR generation.

Runtime Effects still won't allow a lot of ES3 things (the Pipeline
stage will reject unsupported statement types, SkVM doesn't support most
non-ES2 constructs, etc). However, this change will give us the ability
to test many more features involving arrays and structs that previously
were off-limits due to ES2 restrictions, and will shore up some
legitimate gaps in our testing. This is a useful starting point to allow
for improved test coverage.

Change-Id: I4a5bc43914e65fc7e59f1cecb76a0ec5a7f05f2f
Bug: skia:11209
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402157
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-04-28 19:59:36 +00:00
Brian Osman
bf3e9e9591 Remove sample-with-matrix from SkSL
This is completely unused - GrMatrixEffect is the only thing that deals
with matrix transforms on child sampling. Removing this makes everything
simpler to reason about.

Change-Id: I555a3fd937c064f2480b149a6d4d8e36f7ee69bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402176
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2021-04-28 15:44:35 +00:00
John Stiles
c4e9588621 Improve DSL CPP support for global variables.
Global variables which can be calculated in C++ code are now written as
constant values in the DSL, instead of performing the same logic
redundantly in the shader.

In some cases this can be fairly significant, e.g. RectBlurEffect has
a global with the expression
    abs(rect.x) > 16000 || abs(rect.y) > 16000 ||
    abs(rect.z) > 16000 || abs(rect.w) > 16000

Change-Id: I84221f60a4986b3225afcf91ef95cdcfc941b4b7
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401437
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-27 21:59:41 +00:00
John Stiles
ec871bf353 Fix global variables which call built-in functions.
When emitting a built-in function call, we need to honor the `fCPPMode`
state. Previously we were rewriting the call into a DSL call even in C++
mode.

We also now spell out Fract's complete namespace, to avoid a name
collision with MacTypes.h:

https://opensource.apple.com/source/CarbonHeaders/CarbonHeaders-18.1/MacTypes.h.auto.html
    typedef SInt32                          Fract;


Change-Id: I752b7816a64a9b2b2c79d92fe46cd774e1bab96a
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401678
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-27 21:54:31 +00:00
Brian Osman
169c8903be Reland "Add coords parameter to all .sksl test files used as runtime effects"
This is a reland of 22dcb5fd7e

Original change's description:
> Add coords parameter to all .sksl test files used as runtime effects
>
> Convert to use the newer MakeForShader factory, which requires this.
>
> Change-Id: Ifaf6054054027c78f3f3fe15596e435e0f79b877
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399336
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>

Bug: skia:11919
Change-Id: I5f745c54b2bc3712f2281db6e067345903e81931
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401836
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-27 20:47:31 +00:00
Joe Gregorio
67baf2dd48 Revert "Add coords parameter to all .sksl test files used as runtime effects"
This reverts commit 22dcb5fd7e.

Reason for revert: Lot's of red Android and Win bots.

Original change's description:
> Add coords parameter to all .sksl test files used as runtime effects
>
> Convert to use the newer MakeForShader factory, which requires this.
>
> Change-Id: Ifaf6054054027c78f3f3fe15596e435e0f79b877
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399336
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>

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

Change-Id: I0fa844c6cf985d16e72c7f26aa217752612dcfc1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401077
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
2021-04-26 19:40:30 +00:00
Brian Osman
22dcb5fd7e Add coords parameter to all .sksl test files used as runtime effects
Convert to use the newer MakeForShader factory, which requires this.

Change-Id: Ifaf6054054027c78f3f3fe15596e435e0f79b877
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399336
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-04-26 18:34:38 +00:00
John Stiles
e3fa745a5a Add support for atan() to DSL.
Change-Id: I00cc1e89fd85fdc0ce0860fcb35ececd0eaec50a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400540
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-04-26 15:18:18 +00:00
John Stiles
b9fc6e45c2 Support 'layout(when)' expressions in DSL C++.
This required some changes to how we name variables in DSL.
When-expressions are designed to expect a local C++ variable with the
same name as the layout key. This constraint means our DSLVar variables
CANNOT have the same name as the layout key. Now, all DSL variables are
given a prefix. We try to keep the code tidy by using just a leading
underscore as the prefix, where it's safe to do so. (The C++ naming
rules put some underscore-names out of bounds, but underscore followed
by a lowercase letter is safe.)

Change-Id: Iaa8878042329b9909096f05712d5cf636ea01822
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400623
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-26 15:17:38 +00:00
John Stiles
1dcf46359c Fix sk_Caps support in DSL C++ code generator.
This was almost right, but was missing the trailing () to make a
function call.

Change-Id: I1215a97bb0ac39aceca8ff6bea70af8ff572ef84
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400541
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-04-23 22:20:13 +00:00
John Stiles
83b0c830c6 Add support for built-in functions to DSLCPPCodeGenerator.
Change-Id: Idf65ff46cd75d23f550c8e763cf3618ec7501b38
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398877
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-04-22 21:56:22 +00:00
John Stiles
d85d720800 Add support for sample() to DSLCPPCodeGenerator.
This CL also removes some vestiges of the kSampler type, which hasn't
been used in .fp files for a long time.

Change-Id: Iaca1d0c6e77ad2df2b6c5dacd1c68079d6dd5cf2
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398738
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-04-22 21:13:13 +00:00
John Stiles
47b48bc2be Reland "Add support for uniforms and layout(key)s to DSLCPPCodeGenerator."
This reverts commit 289da86e37.

Reason for revert: relanding

Original change's description:
> Revert "Add support for uniforms and layout(key)s to DSLCPPCodeGenerator."
>
> This reverts commit f33b061e3b.
>
> Reason for revert: Google3 roll and wasm build
>
> Original change's description:
> > Add support for uniforms and layout(key)s to DSLCPPCodeGenerator.
> >
> > Change-Id: I77c386e3d72fb4a5986e5efb8bc9d409200534d1
> > Bug: skia:11854
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398457
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I006ece639fa6051ff6ef1c496e648db9d5d0b30a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11854
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399498
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>


Bug: skia:11854
Change-Id: I1a4a4db471e2ad0b169b2b77784ca17e6286fbd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400036
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-22 20:04:32 +00:00
John Stiles
956802335d Reland "Reland "Implement statements and expressions in DSL C++ code generator.""
This is a reland of c412688798

This CL lands the code changes but not the dm test, which is causing
link errors. Tests will be relanded as a separate CL, at
http://review.skia.org/400097

Original change's description:
> Reland "Implement statements and expressions in DSL C++ code generator."
>
> This is a reland of 16cbfb41df
>
> Tests now rely on `shaderDerivativeSupport` and `integerSupport` as
> proxies to indicate ES3 support. The SwitchStatement test has been
> adjusted to hopefully confuse fewer compilers.
>
> Original change's description:
> > Implement statements and expressions in DSL C++ code generator.
> >
> > This CL removes the bulk of the existing C++ code generator, especially
> > all the complex format-string assembly code. It has been replaced with
> > actual DSL code generation. Simple IR can now be successfully translated
> > to a working DSL fragment processor.
> >
> > This CL also adds a simple test harness which is patterned after the
> > existing SkSLTest; it renders a pixel, reads it back, and fails the test
> > if the result isn't solid green (RGBA=0101).
> >
> > This CL doesn't implement every feature. Some obvious gaps include:
> > - Sampling from children
> > - Uniforms/inputs of any kind
> > - Function calls of any kind
> >
> > Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> > Bug: skia:11854
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> Bug: skia:11854, skia:11891
> Change-Id: I91363e31f34611d15ae350b52d6fc459feeace9c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399076
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

Bug: skia:11854
Bug: skia:11891
Change-Id: Ib1f08256c84d1da2130e0b61356f72435dc0a5a8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399740
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-04-22 20:01:33 +00:00
John Stiles
126128b37c Revert "Reland "Implement statements and expressions in DSL C++ code generator.""
This reverts commit c412688798.

Reason for revert: fix Google3 roll and wasm build

Original change's description:
> Reland "Implement statements and expressions in DSL C++ code generator."
>
> This is a reland of 16cbfb41df
>
> Tests now rely on `shaderDerivativeSupport` and `integerSupport` as
> proxies to indicate ES3 support. The SwitchStatement test has been
> adjusted to hopefully confuse fewer compilers.
>
> Original change's description:
> > Implement statements and expressions in DSL C++ code generator.
> >
> > This CL removes the bulk of the existing C++ code generator, especially
> > all the complex format-string assembly code. It has been replaced with
> > actual DSL code generation. Simple IR can now be successfully translated
> > to a working DSL fragment processor.
> >
> > This CL also adds a simple test harness which is patterned after the
> > existing SkSLTest; it renders a pixel, reads it back, and fails the test
> > if the result isn't solid green (RGBA=0101).
> >
> > This CL doesn't implement every feature. Some obvious gaps include:
> > - Sampling from children
> > - Uniforms/inputs of any kind
> > - Function calls of any kind
> >
> > Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> > Bug: skia:11854
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> Bug: skia:11854, skia:11891
> Change-Id: I91363e31f34611d15ae350b52d6fc459feeace9c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399076
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

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

Change-Id: I71a8cf31e8a013b7a2a0d10f0ad3bc3893ea07ea
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11854
Bug: skia:11891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399499
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-22 13:45:00 +00:00
John Stiles
289da86e37 Revert "Add support for uniforms and layout(key)s to DSLCPPCodeGenerator."
This reverts commit f33b061e3b.

Reason for revert: Google3 roll and wasm build

Original change's description:
> Add support for uniforms and layout(key)s to DSLCPPCodeGenerator.
>
> Change-Id: I77c386e3d72fb4a5986e5efb8bc9d409200534d1
> Bug: skia:11854
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398457
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

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

Change-Id: I006ece639fa6051ff6ef1c496e648db9d5d0b30a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399498
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-22 13:43:20 +00:00
John Stiles
f33b061e3b Add support for uniforms and layout(key)s to DSLCPPCodeGenerator.
Change-Id: I77c386e3d72fb4a5986e5efb8bc9d409200534d1
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398457
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-21 20:18:07 +00:00
John Stiles
c412688798 Reland "Implement statements and expressions in DSL C++ code generator."
This is a reland of 16cbfb41df

Tests now rely on `shaderDerivativeSupport` and `integerSupport` as
proxies to indicate ES3 support. The SwitchStatement test has been
adjusted to hopefully confuse fewer compilers.

Original change's description:
> Implement statements and expressions in DSL C++ code generator.
>
> This CL removes the bulk of the existing C++ code generator, especially
> all the complex format-string assembly code. It has been replaced with
> actual DSL code generation. Simple IR can now be successfully translated
> to a working DSL fragment processor.
>
> This CL also adds a simple test harness which is patterned after the
> existing SkSLTest; it renders a pixel, reads it back, and fails the test
> if the result isn't solid green (RGBA=0101).
>
> This CL doesn't implement every feature. Some obvious gaps include:
> - Sampling from children
> - Uniforms/inputs of any kind
> - Function calls of any kind
>
> Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> Bug: skia:11854
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

Bug: skia:11854, skia:11891
Change-Id: I91363e31f34611d15ae350b52d6fc459feeace9c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399076
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-04-21 20:17:07 +00:00
John Stiles
60191e0502 Revert "Implement statements and expressions in DSL C++ code generator."
This reverts commit 16cbfb41df.

Reason for revert: using ES3 features, breaks on ANGLE ES2 bots

Original change's description:
> Implement statements and expressions in DSL C++ code generator.
>
> This CL removes the bulk of the existing C++ code generator, especially
> all the complex format-string assembly code. It has been replaced with
> actual DSL code generation. Simple IR can now be successfully translated
> to a working DSL fragment processor.
>
> This CL also adds a simple test harness which is patterned after the
> existing SkSLTest; it renders a pixel, reads it back, and fails the test
> if the result isn't solid green (RGBA=0101).
>
> This CL doesn't implement every feature. Some obvious gaps include:
> - Sampling from children
> - Uniforms/inputs of any kind
> - Function calls of any kind
>
> Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
> Bug: skia:11854
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

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

Change-Id: I4f3e7667bf1e3a5539d0248b6c47d9ae2296aa88
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398739
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-20 19:59:37 +00:00
John Stiles
16cbfb41df Implement statements and expressions in DSL C++ code generator.
This CL removes the bulk of the existing C++ code generator, especially
all the complex format-string assembly code. It has been replaced with
actual DSL code generation. Simple IR can now be successfully translated
to a working DSL fragment processor.

This CL also adds a simple test harness which is patterned after the
existing SkSLTest; it renders a pixel, reads it back, and fails the test
if the result isn't solid green (RGBA=0101).

This CL doesn't implement every feature. Some obvious gaps include:
- Sampling from children
- Uniforms/inputs of any kind
- Function calls of any kind

Change-Id: Ib80c23fe1ba4453f7c3cb43b65f93c5ea0deb709
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396757
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-20 19:28:36 +00:00
John Stiles
49b409e680 Simplify for init-stmts at the IR level.
Most backends don't like init-stmts with multiple VarDeclarations inside
them, so we no longer emit IR that does this. This lets us avoid doing
backend-level fixups.

Change-Id: Ide839de18953a73e0f9c7a690df59a7bc3523f89
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398221
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-04-20 14:38:36 +00:00
Brian Osman
8f1dff6a9e Remove layout(marker) from runtime effect SkSL
This is another strange, experimental feature that clutters the
implementation and isn't used by anyone (to my knowledge).

Change-Id: I538b7eca0cd28aab32f4739b23459731ade9105e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398226
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-04-19 18:48:45 +00:00
Brian Osman
e49703faf2 Remove custom SkVertices data and runtime effect varying support
This was an experimental feature. It worked (but only the GPU backend).
It was never adopted or used by anyone, to my knowledge. It's a large
amount of code, and a strange corner of SkSL for users to stumble into.

Bug: skia:10680
Change-Id: I0dda0364bce7dbffa58c32de4c7801ec2a6bc42e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398222
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-19 17:49:25 +00:00
John Stiles
8e2a84be3d Fix for fuzzer-discovered error in SPIR-V compilation.
Unused InterfaceBlocks were not added to the ProgramUsage map. The
ProgramUsageVisitor now makes sure to account for them during its
initial scan.

Change-Id: If3afac8e954c5b685ddc6b63b0f771d8c0b8f207
Bug: oss-fuzz:33405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398016
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-19 15:55:04 +00:00
Brian Osman
102d6e5a63 Add test case for sample() with color and various forms of coords
Demonstrates a bug that was introduced here:
  https://skia-review.googlesource.com/c/skia/+/396021

Bug: skia:11867
Change-Id: Iaa6521bdeb4c05b0b91f4df43505e9983106f964
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397148
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-15 20:34:42 +00:00
Brian Osman
cbb60bd0b0 Add runtime color filter and shader modes to the SkSL compiler
These enforce stricter rules about the signature of main, and each one
uses a separate pre-include module. That prevents color filters from
being able to reference sk_FragCoord (or coords passed to main) at all.
It also limits the versions of sample() that are exposed.

In the new world, an effect created for a specific stage of the Skia
pipeline can only be used to create instances of that stage (SkShader or
SkColorFilter). For now, SkRuntimeEffect::Make uses kRuntimeEffect,
which continues to be more lenient and allow creation of either shaders
or color filters from a single effect. After we migrate all clients, we
can deprecate and then delete that mode.

Bug: skia:11813
Change-Id: I0afd79a72beeec84da42c86146e8fcd8d0e4c09f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395716
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-15 13:30:59 +00:00
John Stiles
c5d860877e Add unit test for for-loop with multiple variables.
We lacked test coverage for this case, and this was broken when compound
VarDeclarations were split from one Statement into several.

Change-Id: I561b4d8acc0bfa01161d873a0c022ec58e316903
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396817
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-04-14 20:23:35 +00:00
John Stiles
82ab3409f7 Fork CPPCodeGenerator into a DSL-based version.
The long term goal is for the DSLCPPCodeGenerator to replace the
CPPCodeGenerator entirely, but we will need both to coexist while DSL is
still under development.

Currently, the DSLCPPCodeGenerator is cloned from CPPCodeGenerator and
emits almost exactly the same code (it adds a comment at the top to
distinguish its output). Its output will change in followup CLs.

This CL also allows skslc to recognize the `_dsl.cpp` output suffix and
generate code using DSLCPPCodeGenerator instead of CPPCodeGenerator.
This allows test DSL FPs to be created for inspection.

Change-Id: If5136279c307ea53a9df3a292caa18344c1eb259
Bug: skia:11854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396096
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-13 22:01:09 +00:00
Brian Osman
962e6140ac Add ivec GLSL type aliases to runtime effects
Also add unit test of all GLSL type aliases.

Bug: skia:10679
Change-Id: I93e21621c11adfe3f114d0c55fb8043518e62696
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395718
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-12 16:55:41 +00:00
John Stiles
d02615698e Fix assertion when performing type-coercion with enums.
In http://review.skia.org/393397, I replaced a Constructor::Convert call
with a call directly to ConstructorCompoundCast::Make.

This worked fine if the input expression was actually a compound, but
if it was not, the code would assert/crash. The fuzzer detected this
error right away. (Enums are not considered to be a scalar, a vector or
a matrix in SkSL.)

Change-Id: Ie0df4c5771ff4f4d8f5251d4703e9c3516b6baad
Bug: oss-fuzz:33113
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395720
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-04-12 16:40:31 +00:00
John Stiles
97186dc957 Fix various fuzzer-generated tests.
Two of the tests are now (correctly) detected as invalid code, but were
previously checked into /shared/ because they compiled when the fuzzer
first reported them (that is, after the crash was fixed). These have
been moved into the /errors/ test folder.

One of the tests was only generating an error because its main function
was named `a` instead of `main`, so I renamed it to `main`.

Change-Id: I1a2346fb16e304b0c66ff377a3f9bf7e7ee89ba9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394899
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-12 13:44:59 +00:00
John Stiles
d3a1df8da7 Remove sk_Height and sk_Width.
These builtins have been obsolete for a long time; they were only
implemented on the GLSL backend. These values are provided in
u_skRTHeight and u_skRTWidth instead. (See GrGLSLProgramBuilder's
addRTHeightUniform and addRTWidthUniform.)

Change-Id: I8cceca348cbf9071939618f913693c316d35dbc6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395001
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-09 21:01:27 +00:00
John Stiles
2de947e780 Fix broken Enum unit test.
This test fell into disrepair when constant propagation was removed; it
assumed that @if and @switch would work on all unchanging variables, not
just those with the `const` modifier.

Change-Id: Ie9c1816f9c0852fdea998e4e156047e4cca9ad5b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395000
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-04-09 19:07:27 +00:00
John Stiles
3509210622 Implement array == and != on SPIR-V.
GLSL (post-ES2) allows array comparison: http://screen/8gryPvb9T7gndyb

Unfortunately, because ES2 does not support array comparisons, we can't
add this test to the dm test suite.

Change-Id: I06b71683e49b2631669cff801dc647951a81a299
Bug: skia:11849
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394162
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-09 15:33:57 +00:00
John Stiles
781496ffe5 Add unit test for array of zero or negative size.
Change-Id: Icc710e414388e4026a5e9819a53b8dac8ee0a2d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394896
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-09 14:38:47 +00:00
John Stiles
d47330feed Add support for struct constructors to SkSL.
This is allowed in OpenGL ES2, and its absence in SkSL has been a pain
point for new users adopting Runtime Effects.

Change-Id: Id2ed78261a2cd2b14b49ad22cb74cdc9e0905f8a
Bug: skia:11368
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393418
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-09 13:54:16 +00:00
John Stiles
bc5c2a0ab9 Implement struct == and != on SPIR-V.
OpenGL ES2 allows structs to be compared: http://screen/6KnX4ZfkdLtqDWv
This already worked in GLSL, Metal and SkVM.

Change-Id: Iaf7029c0c1ea9d447348c8280a2788f0d36befad
Bug: skia:11846
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393598
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-04-08 17:17:34 +00:00
Brian Osman
5c62582874 Require that runtime effect child variables are 'uniform'
Bug: skia:11374
Change-Id: I63d605eabbe514a0469d00d8a671969874f3edd4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393081
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-07 13:16:28 +00:00
Brian Osman
601abfacc7 Move 'shader' usage tests to an SkSL golden file
Also adds tests of non-uniform shader declarations. These are currently
allowed, but will be detected as an error in the next CL.

Bug: skia:11374
Change-Id: I3fee0a0c97ae590f7bc6952cb367f7e94436b891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393080
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-06 19:20:16 +00:00
John Stiles
5abb9e1426 Represent matrix resizes as a dedicated expression type.
This shook out a long-standing bug; constant folding would treat a
matrix resize as if the cells not covered by the original matrix were
all zero. This is wrong; GLSL populates the unknown cells with an
identity matrix. We actually tested for the wrong behavior, so the tests
were updated to match the correct behavior, and an equivalent test was
added that does not constant-fold (to verify that our constant folder
matches reality).

Change-Id: I03df10ce646fbef0a36e9c1a841a7637182de122
Bug: skia:11032
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392916
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-06 18:35:06 +00:00
John Stiles
0a12b85f16 Add regression test for oss-fuzz finding.
The actual fix happened at prior CL http://review.skia.org/392197, which
reworked how vector-cast constructors function.

Change-Id: Ifb71ec913b349e65d38458dc615441e7a73efddc
Bug: oss-fuzz:32851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392841
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-04-05 19:39:16 +00:00
Brian Osman
1f56479d6e Add test using 'fragmentProcessor' in runtime effects
Allowed today, will soon be an error.

Bug: skia:11813
Change-Id: I5c13de7657fa85f13fa6d80e1d890225d8a3e868
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392439
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-05 14:02:16 +00:00
John Stiles
e1182785be Create a diagonal-matrix Constructor class.
This constructor takes a single argument and splats it diagonally across
an otherwise-zero matrix. These are also sometimes referred to as a
uniform-scale matrix.

Change-Id: I1ed8140f55f5cad4029015807b220d6475401daa
Bug: skia:11032
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/390716
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-31 18:00:42 +00:00
John Stiles
8037c9e2ed Add test for folding of == and != for arrays.
We don't directly support this today at all. In practice, though, simple
constant arrays are detected as equal in the constant-folding pass
because they hit the `x == x` self-equality check (using
`IsSameExpressionTree`).

This does not work for our inequality tests, though, so those do not
fold.

Change-Id: I6730a9a2d1da9ac613ee58889d651f3ff65b1d2d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/391057
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-03-31 15:32:03 +00:00
John Stiles
fabed8bb79 Fix fuzzer-discovered error with variable declarations.
As soon as a single VarDeclaration is successfully created, its Variable
is added to the current symbol table. However, if a variable-declaration
line declared several variables in a row, we would stop if ANY of the
declarations contained an error and discard the entire statement, but
would continue processing the rest of the program. This left us in a
position where some Variables existed in the SymbolTable with valid,
reachable names, but their corresponding VarDeclaration statement had
been thrown away as erroneous. Since Variables point back to
VarDeclarations for their initialValues, this gave us a stale pointer.
Any future reference to that variable name which could trigger an
access to its initialValue would read from this dead pointer.

This CL fixes the conversion of VarDeclarations so that we no longer
throw away any VarDeclarations associated with a successfully-parsed
Variable.

Change-Id: If8ec3c160933e48a0e1f36414234b3a849d8978c
Bug: oss-fuzz:32587
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389636
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-03-29 14:35:09 +00:00
John Stiles
2febb5b423 Add SkSL test for vector-scalar math.
We had constant-folding tests for vector-scalar arithmetic, but didn't
have an equivalent SkSL unit test for vector-scalar arithmetic that
actually needs to be computed at runtime.

This exposes two SPIR-V bugs: one was previously known, but the other
is a new discovery.

Change-Id: I28737128f20b445797c6c29872335d05f94cc95c
Bug: skia:11267, skia:11788
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388739
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-25 23:30:34 +00:00
John Stiles
0c9d888748 Run SkSL inliner tests as part of dm.
Previously, these tests were never actually executed, only read during
code review. They are now properly tested for correctness whenever dm
is run. Non-ES2 compliant statements (do/while/switch) are unfortunately
excluded here, as they are not compatible with Runtime Effects yet.

Change-Id: I965c782baad6f8dd3961a400ae791fb2c1f844d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389296
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-25 22:58:54 +00:00
John Stiles
85749c0b22 Add caps bit for RewriteMatrixVectorMultiply.
On ARM GPUs, the Vulkan driver does not honor relaxed precision when
evaluating SpvOpMatrixTimesVector. This leads to reduced performance
(compared to GLSL, where mediump matrices and floats do evaluate all
intermediate values at mediump).

This caps bit will be enabled on ARM GPUs and, in a followup CL, will
be used to toggle a workaround where `m*v` is rewritten as the sum of
(m[0]*v[0] + m[1]*v[1] + ... + m[N]*v[N]).

Change-Id: I310fa73639b6498552c9672e76860f2eded15d0a
Bug: skia:11769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388459
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-23 22:40:02 +00:00
Brian Osman
d1f3b97a64 Remove sk_SampleMask support from SkSL
Change-Id: I22837a4921238749664217e595d24d196503534d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388096
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-23 14:37:28 +00:00
John Stiles
1b843bdcb7 Fix up outdated tests.
Removed error tests which no longer test anything:
- UseWithoutInitialize...: we no longer track variable use-before-init.
- InlineDivideByZero: we no longer constant-fold the result of an
  inlined function call into its parent statement.
- Unreachable: we no longer report unreachable statements.

And fixed some minor test-case issues:
- StaticSwitchConditionalBreak: we still check this, but the function
  was being dead-stripped before this check could run. Renamed to main.
- OssFuzzXxxxx: these cases no longer report errors, but they are still
  valuable as regression tests; moved to `shared/`.

Change-Id: Iade3cff821dc998cacfd02f62d3ac4625e48904c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387820
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-22 21:11:47 +00:00
John Stiles
95eadd32c1 Rename 'unpremul_float' to 'unpremul.'
Now that name mangling is implemented, we don't need to worry about name
collisions when high-precision types are disabled.

Change-Id: Ic0def3e629deeb804745ff683826946947c898e3
Bug: skia:10851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387758
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-22 17:34:16 +00:00
John Stiles
ece1d794b9 Mangle function names in GLSL.
This will be implemented in Metal and SPIR-V in followup CLs.

Change-Id: I397b4db40b15dd54cf1d8a17f414c3fe184b48d2
Bug: skia:10851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387638
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-22 17:18:26 +00:00
John Stiles
e67bd1371e Migrate VarDeclaration conversion out of IRGenerator.
Change-Id: If5fb4f99d327bb429f60e8d6c526720dd02b0928
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386800
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-22 14:25:26 +00:00
John Stiles
bff24abab8 Disallow inlining a function with out-parameters.
It is difficult to do this both efficiently and correctly while honoring
GLSL semantics (which require the lvalues to be kept distinct, even when
they point to the same variable). We could make it work by making copies
of every out parameter in each direction (going in for inouts, and
coming out for outs and inouts).

However, this could be self-defeating if it makes it harder for the
driver to track variable lifetimes. Simply opting out of inlining these
functions entirely seems like the best tradeoff; let the driver optimize
them if it can, and we can enjoy reduced complexity in the SkSL inliner.

Change-Id: I62f7b4550cc181cfe789e4f2ff4e408ba1baf9cb
Bug: skia:11326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370257
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-17 18:04:00 +00:00
Ethan Nicholas
e0707b7075 No longer passing the results of OpAccessChain to function calls
It turns out it is not legal to pass the results of OpAccessChain as a
function argument, for... reasons. This CL switches us over to passing
the argument via a temp variable instead.

Bug: skia:11748
Change-Id: Ib5e86c1d000655ebd7bb62ceea6a27b823808645
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-03-17 16:37:00 +00:00
John Stiles
dc20847579 Disallow inlining functions containing early returns.
This allows us to remove 100 LOC from the inliner and is very unlikely
to affect any existing benchmark. We don't have any evidence to support
the idea that a one-iteration `for` loop with `continue`-based exits
will be any faster than a standard function call on any existing GPU.
Our fragment processors are generally written to avoid early returns,
in large part to avoid hitting this path.

This drastically impacts BlendEnum.sksl (which can no longer flatten out
a switch over every blend function in SkSL) but is otherwise a wash.

See: http://go/optimization-in-sksl-inliner suggestion 4(a)

Change-Id: I1f9c27bcd7a8de46cc4e8d0b9768d75957cf1c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385377
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-17 16:29:00 +00:00
John Stiles
2810beed23 Improve do-while test in CanExitWithoutReturningValue.
This put the coverage for do-while loops on par with for loops.

Change-Id: I53e0d733edd02a6a139792a8d74c68116453e5ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385500
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-17 14:50:56 +00:00
John Stiles
f10eff363b Fix misdetection of dead global variables.
A global variable should be considered "dead" if it's never written and
never read. The previous code checked if it was never written OR never
read, which is not the same.

This would generate GLSL/Metal that didn't compile. In SPIR-V, it would
SkASSERT, then crash, during codegen. The fuzzer was able to detect the
SPIR-V issue, but it was wrong in all three cases.

Change-Id: Id59a2499eb5baa3839b93826bfbc24191bfd490b
Bug: oss-fuzz:32005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385280
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-03-16 16:02:03 +00:00
Brian Osman
8c595fed7c Change sksl tests to avoid SPIR-V validation errors
'in' variables without locations aren't allowed. Use uniforms instead.

Bug: skia:11738
Change-Id: Ic066106deb7409cff154b4be7cfb3e03a7025c7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385000
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-16 13:39:53 +00:00
Brian Osman
fc4b9919d3 Force global initializers to be constant expressions
Prevents us from accepting code that can't be correctly transformed to
GLSL, like:

  uniform float x;
  float y = x;

(Previously, writing code like that in a runtime effect would
effectively produce the exact same code all the way through to GLSL, and
the driver would fail to compile it).

Bug: skia:11336
Change-Id: Iaa797587c4a4a7289ed59ce2736cf0bf0fc5bca3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384698
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-15 16:54:05 +00:00
John Stiles
97d8d7fce0 Remove obsolete TODO.
The referenced bug is closed, and the compiler does correctly emit an
error here.

Change-Id: I824f7819a1e077163f528ac503f3743aab5e58a1
Bug: skia:11322
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384697
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-03-15 14:43:25 +00:00
Ethan Nicholas
2f4652f309 Revert "Fixed a number of spots where we should have been using RelaxedPrecision"
This reverts commit a04692f69e.

Reason for revert: Angry Vulkan bots.

Original change's description:
> Fixed a number of spots where we should have been using RelaxedPrecision
>
> Our SPIR-V output was missing many RelaxedPrecision decorations, which
> was presumably impacting performance.
>
> Change-Id: Iee32d4a42f37af167fe0e45f3db94c2142129695
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384178
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

TBR=egdaniel@google.com,brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: If4fe945cb363c9b61b5a4abfde649a437689d2eb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384217
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-03-12 18:48:57 +00:00
Ethan Nicholas
a04692f69e Fixed a number of spots where we should have been using RelaxedPrecision
Our SPIR-V output was missing many RelaxedPrecision decorations, which
was presumably impacting performance.

Change-Id: Iee32d4a42f37af167fe0e45f3db94c2142129695
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384178
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-03-12 18:00:56 +00:00
John Stiles
f3a28db703 Eliminate control-flow analysis.
We no longer derive a performance benefit from this pass in practice,
and it is a very expensive compilation step. It is also prone to fuzz-
related errors.

Doc: http://go/optimization-in-sksl

Change-Id: Ief08ffac659a8fe7fe92c92b9a5da14c9f713bc2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381261
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-11 13:24:54 +00:00
John Stiles
0dd1a77e12 Add noinline keyword to SkSL.
As you might expect, a function tagged with `noinline` will never be
considered as a candidate for inlining.

Change-Id: Ia098f8974e6de251d78bb2a76cd71db8a86bc19c
Bug: skia:11362
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382337
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-10 15:39:48 +00:00
Brian Osman
86e85537fe Test that we propagate 'const' to the GPU backend of runtime effects
Currently, only one of three uses (local variables) does this correctly.

Bug: skia:11716
Change-Id: Iad11e8e5998fcc7caee4d438e0558c5d4e2b1821
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382277
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-09 23:26:00 +00:00
John Stiles
51d33982ad Add Convert/Make factory functions to IndexExpression.
Change-Id: I7a7874e58bf53978afce8a41b26092406b6490ed
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380360
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-08 15:55:59 +00:00
John Stiles
6fb520fcd5 Improve optimization of prefix expressions at IR time.
- Values from constant variables are folded in when it helps, e.g.:

    const bool SHINY = true;
    const float SHININESS = 2;
    if (!SHINY) {  // <-- optimizes directly to `false`
       param = -SHININESS; // <-- optimizes to `-2`

- Doubled-up logical-not and negation are stripped:

    y = -(-x); // <-- optimizes to `y = x;`
    b = !!a; // <-- optimizes to `b = a;`

Removal of doubled-up negation and logical-not was actually never
implemented in the constant-propagation phase; I just noticed it while
I was here and thinking about it.

Change-Id: Ie28bb9b5af91376f03d926e26e37f4a131bbf550
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379298
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-05 21:41:55 +00:00
John Stiles
28054added Optimize ternary tests that check a const variable.
This enables the ternary to be optimized away in code like:
   const bool SHINY = true;
   color = SHINY ? add_shine(x) : x; // to --> `color = add_shine(x);`

Without constant propagation.

Also, I added a unit test for ternary expression simplification; I
wasn't able to find an existing one.

When the optimization flag is disabled, this CL actually removes the
optimization of `true ? x : y` --> `x` entirely; previously, this
substitution would be made regardless of optimization settings.

Change-Id: I93a8b9d4027902d35f8a19cfd6417170b209d056
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379297
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-05 21:41:05 +00:00
John Stiles
b3dcbb12ef Detect functions that fail to return a value, without using CFG.
This check now runs at function finalization time, before constant
propagation has occurred; this affected the "DeadIfStatement" test.

Our detection isn't smart enough to realize that a loop will run zero
times, so it treats `for` and `while` loops as always running at least
once. This isn't strictly correct, but it actually mirrors how the CFG
implementation works anyway. The only downside is that we would not flag
code like `for (i=0; i<0; ++i) { return x; }` as an error.

Change-Id: I5e43a6ee3a3993045559f0fb0646d36112543a94
Bug: skia:11377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-04 22:47:05 +00:00
John Stiles
2b6ec98a82 Disallow unscoped for blocks which declare a variable.
This should be legal, and we support this, but some versions of Android
do not: http://screen/3bkQewHF3xUMn5v There's no point in allowing
these shaders to exist; they can't compile on real-world clients, and
these vardecls are borderline meaningless (as the variables being
declared aren't reachable by any other statements).

Change-Id: Ie1351933c90caee9124eeab8983364ec030b2653
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379584
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-04 17:04:40 +00:00
John Stiles
4633c9149b # Enter a description of the change.
Reland "Migrate if-statement simplifyStatement logic to IfStatement::Make."

This reverts commit 7e685f0377.

Reason for revert: fixed SkSLBench perf test

Original change's description:
> Revert "Migrate if-statement simplifyStatement logic to IfStatement::Make."
>
> This reverts commit e4da7b672f.
>
> Reason for revert: breaks SkSLBench perf test
>
> Original change's description:
> > Migrate if-statement simplifyStatement logic to IfStatement::Make.
> >
> > This performs essentially the same simplifications as before, just at
> > a different phase of compilation.
> >
> > Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
> > Bug: skia:11343, skia:11319
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I0051188ffe69426904066eb60a932435efdc2af8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11343
> Bug: skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379062
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

Bug: skia:11343
Bug: skia:11319
Change-Id: I74cc3295004133e9fdcf16e388106eb83603f526
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379063
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-03 22:21:36 +00:00
John Stiles
7e685f0377 Revert "Migrate if-statement simplifyStatement logic to IfStatement::Make."
This reverts commit e4da7b672f.

Reason for revert: breaks SkSLBench perf test

Original change's description:
> Migrate if-statement simplifyStatement logic to IfStatement::Make.
>
> This performs essentially the same simplifications as before, just at
> a different phase of compilation.
>
> Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
> Bug: skia:11343, skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

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

Change-Id: I0051188ffe69426904066eb60a932435efdc2af8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11343
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379062
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-03 20:50:37 +00:00
John Stiles
e4da7b672f Migrate if-statement simplifyStatement logic to IfStatement::Make.
This performs essentially the same simplifications as before, just at
a different phase of compilation.

Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
Bug: skia:11343, skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-03 18:12:35 +00:00
John Stiles
2dda50dd50 Add return-value check to the function finalizer.
Rather than have the inliner own this responsibility, the function
finalizer now detects if a function is supposed to return a value but
never actually does. This will allow us to detect this error case even
if the inliner is disabled. The inliner should no longer encounter
functions that claim to return a value but don't, so it will now assert
if one is encountered. (The inliner still has the logic to handle this
case gracefully, just in case.)

The check is currently very simple and doesn't analyze the structure of
the function, so it won't report cases where some paths return a value
and others don't, e.g. this will pass the test:

    int func() { if (something()) return 123; }

(This is good enough to resolve the inliner issue, though, as it only
occurred in functions with no value-returns at all.)

Change-Id: I21f13daffe66c8f2e72932b320ee268ba9207bfa
Bug: oss-fuzz:31469, oss-fuzz:31525, skia:11377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377196
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-03 16:29:22 +00:00
Brian Osman
dc4f8cd7ea Add early_fragment_tests layout qualifier to SkSL
Bug: skia:11356
Change-Id: I16322e6396dc7e7c8c50ba1d39e07311cf3bd346
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376116
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-02 16:45:03 +00:00
John Stiles
0c2d14a1b9 Fix fuzzer-discovered error with inlining.
In http://review.skia.org/375776, an optimization was added to the
Inliner, causing it to skip generation of unnecessary temporary
variables. The fuzzer immediately discovered a flaw in this logic: the
"unnecessary" variable was actually used in the rare case that a
function failed to actually return a value. The inliner didn't detect
this case. Of course, this isn't a valid program either, so now we
report the error and cleanly fail.

Change-Id: I1f201cfd33f45cace3be93765a4e214e43a46e69
Bug: oss-fuzz:31469, oss-fuzz:31525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377101
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-01 20:06:50 +00:00
Brian Osman
a654faaf7f Add permittedLayoutFlags to checkModifiers
For now, just use this to prevent *any* layout qualifiers from appearing
on functions, or their parameters.

Bug: skia:11301
Change-Id: I05d8118c7121048c6ef49695a54e3714a8f8687e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376796
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-26 18:38:31 +00:00
Brian Osman
7b361499c9 Align SkSL const rules more closely with GLSL
This adds Analysis::IsConstantExpression, to determine if an expression
is a constant-expression. It now expands to cover 'const' local and
global variables, because we also enforce that the initializer on those
variables is - in turn - a constant expression.

This fixes 10837 - previously you could initialize a const variable with
a non-constant expression, and we'd emit GLSL that contained that same
pattern, which would fail to compile at the driver level. That should
not be possible any longer.

Bug: skia:10679
Bug: skia:10837
Change-Id: I517820ef4da57fff45768c0b04c55aebc18d3272
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375856
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-26 17:44:11 +00:00
John Stiles
bb8542f086 Fix fuzzer-discovered error with ~ prefix on literals.
The expression `~123` was making a PrefixExpression of type $intLiteral.
It should be converted to type `int` when the ~ prefix is applied.

This change also changes the output from oss-fuzz:27614. Both programs
are essentially nonsense expressions with no real behavior, so this is
fine.

Change-Id: I586be149ce95136fabee72fdd3473814d54948cf
Bug: oss-fuzz:31410
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376620
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-26 17:30:26 +00:00
Brian Osman
029851d951 Remove fragmentProcessor field access
Change-Id: Iafeb13812851271a5262730e9c0642d4469c273f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375020
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-24 21:33:36 +00:00
Brian Osman
a77ed8b382 Add flags for all layout qualifiers, check for duplicates
Now, even if a qualifier has a default value, we will know that it
appeared in the text. We can use that to check for redundant qualifiers
(as is being done here), and in the IR generator to prevent any use of
certain qualifiers, depending on context. (eg, runtime effects, wrong
shader stage, on a parameter declaration, etc.)

Bug: skia:11301
Change-Id: I2cd6ad35c2b4c4d6f87ade97e80aea84dc16ee4b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374616
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-24 16:22:26 +00:00
John Stiles
642cde289d Optimize @switch statements in SwitchStatement::Make.
At IR generation time, this CL limits our optimizations to only
@switch statements. A regular switch statement will only be optimized
during the optimization phase even if the switch-value is a known
compile-time constant. This is done to avoid upsetting our reachability
analysis.

Most of this CL is moving existing logic from SkSLCompiler into
SkSLAnalysis and SkSLSwitchStatement. Although the diffs look large, the
actual changes are very small.

Change-Id: I90920f41bc386dfa7a980ae7510f6681231a5120
Bug: skia:11340, skia:11342, skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372679
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-23 21:36:04 +00:00
Brian Osman
4a015c5092 Require initializers on const variable declarations
Bug: skia:10837
Change-Id: I33da2eb1e723ed04ab62d65c21e54306dd362bed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372677
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-23 20:55:01 +00:00
Brian Osman
4882e9a43d Remove specific blend mode layout qualifiers
These were unused - we always enable the advanced blend equation
extension using "blend_support_all_equations" (if enabling the
extension is required at all).

Change-Id: I95fd6483ec54dfaf983290de95629fe0e86c22e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/373877
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
2021-02-23 15:45:55 +00:00
John Stiles
2b3d1f2086 Remove constant propagation from static-switch test.
Constant propagation might be going away, but static-switches are likely
here to stay. Avoid conflating the two in this test.

Change-Id: If4b6c99c85f124d3bbc20da858693f09f5e4fd59
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374117
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-02-23 15:39:05 +00:00
John Stiles
04ca41acf3 Fix switch optimization pass.
The optimizer now properly recognizes all types of exits from a switch
statement. Break, continue and return are all potential exits and need
to be considered when determining the exit path from the switch.

Previously, dead code elimination was hiding the effects of this bug
from us, but it meant that an optimized switch had the potential to
generate lots of worthless IR nodes which then needed to be detected and
eliminated by the CFG. In particular, this affected the enum form of
blend, causing a catastrophic amount of extra work to be done.

Change-Id: If857e38cadfc016884624ea4db25a273ad3dce5b
Bug: skia:11352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372958
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-23 15:38:24 +00:00
Brian Osman
ffee4766fa SkSL: Allow 'const' on function parameters
Change-Id: I672345116e3b5538c0f7e8c5f2f74aa56bb81e6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372676
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-22 21:40:26 +00:00
John Stiles
66c53b9428 Demonstrate a bug with inlined static switches.
When we detect a static switch, the optimizer finds the matching switch-
case and eliminates all the other switch-cases. It handles case
fall-through by scanning forward and looking for an unconditional break.

However, the inliner has an interesting quirk--it can replace `return`
statements inside of a switch with `continue` statements, since the body
of the inlined function has been wrapped with a for-loop to allow for
early exits. The optimizer does not recognize these continue statements
as exits from the switch (although they certainly qualify), so it
treats continues as fallen-through and keeps emitting switch-cases.

The dead-code elimination pass was actually doing us a favor here and
eliminating the excess code later. A flag was added to disable DCE in
order to reveal the problem in a test.

Change-Id: I8ff19fde5e32d0ab73d7c5411da40cb953a446f5
Bug: skia:11352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372956
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-22 15:05:58 +00:00
John Stiles
463fad21ba Add test for duplicate default switch-case.
Surprisingly, this error is actually caught by our parser, which
interprets the default label in a unique way. From the parser comments:

  "Requiring default: to be last (in defiance of C and GLSL) was a
   deliberate decision. Other parts of the compiler may rely upon this
   assumption."

The comment is true--we don't check for duplicate default switch-case
labels anywhere else in the code, just here in the parser.

We rely on this, so we should have a test for it.

Change-Id: I6df5c565aca4d4b8565b96638dce9504efc39ccc
Bug: skia:11340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372617
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@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-19 20:31:50 +00:00
Brian Osman
58ee89879f Remove "buffer" modifier from SkSL
Change-Id: I4df18946cdb3d9f1f7833461f913f2df94696821
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372197
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-18 21:48:05 +00:00
Brian Osman
9ca38ff4b4 Remove GLSL memory qualifiers from SkSL
These were all unused, and only implemented on one backend.

Change-Id: Ibd2fcef1a971e6c1bd9da0784c5d852a60708484
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372117
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-18 19:57:05 +00:00
Brian Osman
9cb3f98191 Remove PLS support from SkSL
Change-Id: I885149c73be63c223ac88a697ffe046a7f8384d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372116
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-18 19:16:45 +00:00
Brian Osman
2fe67f9680 Disallow initializers on uniform variables
Bug: skia:11335
Change-Id: I88c952cbfe2d2c5920e17675da1674928f37b982
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371480
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-18 01:01:38 +00:00
John Stiles
973009b04b Clean up type coercion code and IRGenerator::checkValid().
When coercing a type, we would previously call checkValid() so we could
detect function-references and type-references, so we could get a nicer
error message.

It turns out that we can just do the "is this a type-reference/
function-reference?" check directly inside coerce() and get the same
improved error messages. Since we should be coercing all our values to
the right type, and type/function-references aren't coercible to
anything, this should catch them all. I don't expect any of these
to survive all the way to the end of IR generation.

(In case one of these types does slip through, I've left the error case
in checkValid, but I've also put in an assertion. If the fuzzer can
make that assertion fire, we are probably missing a call to coerce()
somewhere.)

This cleanup is meant to help migrate coerce() out of IRGenerator.

Change-Id: I031809adf439b1766048768b782c57e7f2494006
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371479
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-17 22:39:48 +00:00
Brian Osman
a5ab63a062 Tweak GlobalVariables.rte test, and adjust .stage output
Adds trivial name mangling to the .stage output, so we can verify that
it's working in all places (declarations, references, etc). Also added
another global variable whose initializer is - in turn - another global.

Bug: skia:11295
Change-Id: Ic220bfae0a6d1eeeba66ade30d3d781af15c5dea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371477
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-17 22:13:19 +00:00
Brian Osman
b41a1f9025 Add runtime effect test for global variables
Includes variables with and without initializers. Note that both the
.skvm and .stage output is incorrect right now. (No declarations for
global variables in .stage, and the initializer is dropped in .skvm).

Bug: skia:11295
Change-Id: Icb6d797616be6a1bc7cbdc9db4fefa7e30c65656
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371143
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-17 18:11:46 +00:00
Brian Osman
6e2dccb979 Runtime Effects: Disallow all operators on arrays or arrays-in-structs
None of these are legal in GLSL ES 1.0. Added a new test that previously
compiled without error. Started out with just assignment and equality,
then realized that sequence and ternary should be blocked, too.

Bug: skia:11323
Change-Id: I02691f819565afabeadbb12cab6c07acf40093f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370880
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-17 18:05:04 +00:00
John Stiles
4a3ec173b3 Add tests to demonstrate SPIR-V error with intrinsics.
When SPIR-V generates function calls to an intrinsic, it assumes that
it can get a pointer to out-parameters referenced by the intrinsic.
This does not account for swizzled out-parameters; these are valid
lvalues, but do not work with getPointer().

The two intrinsics supported by SkSL which have an out-parameter are
frexp and modf, so these tests were fleshed out to trigger the error.
Neither of these are supported in ES2, though, so we cannot test them
via Runtime Effects.

Change-Id: Ib92707a28ba6d1c282d20e29a2a387bddf74ad23
Bug: skia:11052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370116
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-16 15:46:01 +00:00
John Stiles
73e2c8976a Allow repeated params in Metal out-params helper.
The out-param helpers emitted by the Metal code gen (intended to provide
GLSL out-parameter semantics in Metal) emitted bad code if passed the
same variable for two separate out parameters. It would previously
create two parameters in the helper with the same name. The helper
function now omits the name of the second variable in the parameter list
if it is redundant; we already know the caller is passing the same
variable twice.

Change-Id: Ibdc6c02a9e9e4bdb4f4546a25068f2018aa07b10
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370258
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-16 15:45:45 +00:00
John Stiles
92d83b7b9d Add test to demonstrate out-param semantics violation.
GLSL ES2 documentation on out parameters: "Evaluation of an out
parameter results in an l-value that is used to copy out a value when
the function returns."

The inliner does not do any alias checking when inlining an `out` param.
That is, passing the same variable to two separate `out` parameters
would not generate two distinct lvalues in the inlined code; it reuses
the same variable for each out-params in the inlined code.

(Amusingly, our CFG can fully optimize away this test code so it just
returns "red".)

Change-Id: Ib781d2cfdac54f01b6abe159af0c84ff24ff6976
Bug: skia:11326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370256
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-16 14:28:49 +00:00
John Stiles
80b02af6ba Fix assertion when arrays are double-declared.
Multi-dimensional arrays aren't legal in GLSL/SkSL, so this should be
caught and flagged as an error. The parser now verifies that a
variable's type isn't an array-type before accepting a `[` token to
open an array on the variable name.

This CL also refactors the IR generator's `convertArraySize` method to
make sure that various checks are made for all callers. Originally this
restructuring was used to verify array multi-dimensionality, but that
didn't detect errors inside struct declarations (which get no error
checking inside the IR generator) so the IR generator updates no longer
need to check the array dimensions.

Bug: skia:11322
Change-Id: Id33f4bdfb544019ddf995a8196c3c09cfe5a4525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369916
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-12 23:49:04 +00:00
John Stiles
8dabeac58c Improve error reporting for invalid var-decls.
We now interpret any statement of the form `Type identifier...` as a
var-declaration and report errors as such. Previously, if a var-decl
statement generated an error during parse, we'd report errors as if it
were an expression-statement, which meant that slightly-invalid code
could return out-of-context, misleading errors.

Bug: skia:11287
Change-Id: I2c6cf2984760eb34593c80cb30f8c4e007d42027
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370036
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>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-12 22:26:36 +00:00
Brian Osman
6c3b23fcf3 SkSL: Ensure struct declarations contain at least one member
Bug: skia:11314
Change-Id: I66476543462ae378a5bfb6cbd902dfa2f5fc45f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369917
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-12 20:57:56 +00:00
John Stiles
bb1505f2ab Move static-switch and static-if tests out of scanCFG.
We don't need to do these tests every time we run CFG optimization; we
can do them once at the end of optimization, as a separate step.

Change-Id: If0e72fbacb938b62387fd2ffdbf34d1153bf3bd4
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369481
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-02-12 15:32:32 +00:00
John Stiles
ec24154521 Fix SPIR-V bug with swizzled out parameters.
The CL at http://review.skia.org/366399 introduced a bug with
LValue::getPointer. Specifically, getPointer used to return zero when
no pointer is available. (This happens when the LValue is a swizzle.)
That CL changed the error code to -1. However, it did not fix up all
the call sites that checked the return value of getPointer().

This CL fixes up those call sites to use -1 consistently, and adds
TODOs in spots which do not check the result from getPointer() at all
(instead assuming it cannot fail). This will allow swizzled out-
parameters to work in SPIR-V as they did before. (Except in intrinsics,
where they seem to have been broken all along, but those are now marked
with a TODO at least.)

Note that we still do not fully emulate GLSL semantics for out
parameters, as out-parameters should only be copied back to the original
variable at the end of the function call to be fully GLSL compliant.

(This CL also replaces a tuple with a named struct for readability.)

Change-Id: I708dc7a69296a4244ba9ceb85c3e68d1f331bbc9
Bug: skia:11052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368618
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-12 15:21:27 +00:00
Brian Osman
8e756f379c Support structs in runtime effects
Uses the pipeline-stage callback mechanism. It mangles the type name
(with a test to verify that this works), and then calls defineStruct
with the entire SkSL struct definition string.

Bug: skia:10939
Change-Id: If14cf1b11faaa80ad8d4086cdacf68532bac43fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368809
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-11 21:09:15 +00:00
John Stiles
9cfcb07d7a Fix alpha channel in SelfAssignment test.
This was being set to zero instead of one by mistake. Interestingly,
this was undetected by the CPU backend, but appears to matter sometimes
on the GPU side.

Change-Id: If827863f69c140f933696c6ff55c8a7095620c29
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368858
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-11 14:41:15 +00:00
John Stiles
0ac6c15075 Reduce self-assignment expressions (x = x) into just x.
Change-Id: I44356aa781f208f4941dbeb617a30318e36c1081
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367062
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-10 20:00:05 +00:00
John Stiles
a0e407d4b7 Add SkSL test for matrix equality/inequality operators.
Disabled on Adreno 5xx/6xx as the tests do not pass on those GPUs:
http://screen/3Dkgs9syj37cjBV

Change-Id: Ib935d01e8f06dbfe7decd5cc4e52e0688b48be08
Bug: skia:11306, skia:11308
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368805
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-10 18:57:22 +00:00
John Stiles
0bd5578c31 Replace Metal constant keyword with const.
"Constant" is an address space qualifier and can't be applied to a
local variable. "Const" in GLSL (and hypothetically SkSL) is meant to
apply to a constant expression regardless of address space.

Our previous test was not finding any error because the optimizer was
eliminating the constant expressions entirely.

Change-Id: I6cfe8e2a621c79945b33e0166780d81e79890a1b
Bug: skia:11304
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368517
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-10 15:23:25 +00:00
Greg Daniel
4908a24d4b Revert "Add support for matrix == and != in Metal shaders."
This reverts commit c501857188.

Reason for revert: breaking many bots

Original change's description:
> Add support for matrix == and != in Metal shaders.
>
> We need to polyfill an operator== and != when these are first
> encountered in the code.
>
> Change-Id: I539c838ee1871bcb0c4b66abb8a4a0f91146cd4f
> Bug: skia:11306
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368496
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

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

Change-Id: Id583109a0d167c2c58a57644b14cd5f49d670737
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11306
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368801
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2021-02-10 14:25:54 +00:00
John Stiles
c501857188 Add support for matrix == and != in Metal shaders.
We need to polyfill an operator== and != when these are first
encountered in the code.

Change-Id: I539c838ee1871bcb0c4b66abb8a4a0f91146cd4f
Bug: skia:11306
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368496
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-09 22:53:18 +00:00
John Stiles
34c098d7f8 Add SkSL test demonstrating missed optimization with +=.
Currently, SkSL is able to constant-propagate `x = x + constant` into
`x = constant` when the starting value of x is known. However, it is not
able to do the same optimization for `x += constant`. This test
demonstrates that once += is encountered, we lose track of x's value and
can no longer propagate its value.

(This is equally true of all the op-assignment operators, += -=
*= /= etc.)

Change-Id: I3523e96baf9a73982cf3b09f0d23b95adacf106b
Bug: skia:11192
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368248
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-09 19:17:26 +00:00
John Stiles
ecd7c2278f Migrate the last SkSL tests in shared/ to dm.
The leftover tests in shared/ are not easily testable as Runtime
Effects; they do things that ES2 doesn't support or use a feature not
exposed directly by Runtime Effects.

Change-Id: I7ebe170cf713c4a0d2dbef333c1fcbac2410c67f
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367059
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-08 21:37:48 +00:00
John Stiles
a793f4f6fc Add additional cases to folding tests.
These cover new ground; when combined with some additional optimization
work, they can cause crashes in the optimizer that we don't see from any
existing test.

Change-Id: I3958a5522cfe0929d0753e6e617d72e032c7f5a3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367063
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-08 16:17:18 +00:00
John Stiles
c2c1b0c460 Migrate some SkSL swizzle tests to dm.
Change-Id: I2bd6bf9c368359a2b21861c1b6f621040d335111
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367056
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-08 16:16:28 +00:00
Ethan Nicholas
c0f9815609 Added detection for non-constant-folded divide by zero errors in
SkSL.

It would previously catch 1 / 0, but fail to detect x / 0.

Bug: skia:11051
Change-Id: I3adb5942cce03a7ad40a13a8ca5d5a7f2029d6ad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366720
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
2021-02-06 03:13:16 +00:00
John Stiles
abc3b78011 Eliminate left-side comma expressions without side effects.
This takes away one of our gadgets for thwarting dead-code elimination
in unit tests, but it's the right thing to do. Comma expression left-
sides without side effects are clearly dead code.

Change-Id: Iaee490b4a742d06a0a0be94cddaa69a51543d8f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366719
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-05 16:50:33 +00:00
John Stiles
d437478348 Migrate even more SkSL tests to run in dm.
- ResizeMatrix
- ScalarConversionConstructorsES2
- StackingVectorCasts
- StaticIf
- SwizzleBoolConstants

Non-ES2 compatible tests:

- ResizeMatrixNonsquare
- ScalarConversionConstructorsES3

Newly discovered bugs:
- skia:11278 SPIR-V does not support casting non-square matrices

Change-Id: I7feb78f0380d0a9cb328ec61f96a364ad1281432
Bug: skia:11009, skia:11278
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366408
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-05 16:04:43 +00:00
John Stiles
b88b4e1a12 Migrate additional SkSL tests to run in dm.
- MultipleAssignments
- NegatedVectorLiteral
- NumberCasts
- OutParams
- OutParamsTricky (disabled on GPU due to skia:11269)

Change-Id: I87dc9c5019931f3d2dc3aafbe1e02d0eee2e1a05
Bug: skia:11009, skia:11269
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366400
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-02-04 22:12:27 +00:00
John Stiles
e609598fe6 Reject array-typed vardecls and array ctors in strict ES2 mode.
This is enforced by ANGLE in Strict ES2 mode; we need to enforce it as
well.

Change-Id: I6e2f547ad8e0ce817742cf84659764cf6bce38b9
Bug: skia:11270
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366339
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-02-04 19:36:50 +00:00
John Stiles
b44185086e Reland "Improve support for arrays in Metal."
This reverts commit 38df4c8470.

Reason for revert: updated ArrayTypes test for ES2 compatibility

Original change's description:
> Revert "Improve support for arrays in Metal."
>
> This reverts commit dd904af566.
>
> Reason for revert: breaks ANGLE
>
> Original change's description:
> > Improve support for arrays in Metal.
> >
> > Arrays in Metal now use the `array<T, N>` type instead of the C-style
> > `T[N]` type. This gives them semantics much more in line with GLSL,
> > so they can be initialized and assigned like GLSL arrays.
> >
> > This allows the ArrayTypes and Assignment tests to pass, so they have
> > been added to our dm SkSL tests. (ArrayConstructors also passes, but
> > is not ES2-compliant so it is not enabled.)
> >
> > Change-Id: Id1028311963084befd0e044e11e223af6a064dda
> > Bug: skia:10761, skia:10760, skia:11022, skia:10939
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365699
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: If6a18dea7d6a45fa7836e9129bf81c2e536f07e3
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10761
> Bug: skia:10760
> Bug: skia:11022
> Bug: skia:10939
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365976
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

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

Bug: skia:10761
Bug: skia:10760
Bug: skia:11022
Bug: skia:10939
Change-Id: Ia1c4917f5d3c41162d282b3093814d861707ad30
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366144
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-04 17:33:28 +00:00
John Stiles
16a97bb2cb Re-enable SkSL Hex test with constants that fit in 16 bits.
Larger constants caused failures on Adreno 330 (Nexus 5).

Change-Id: I1af1df786f8e4d34f8ec47152875927bfe948190
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366336
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-04 17:15:08 +00:00
John Stiles
38df4c8470 Revert "Improve support for arrays in Metal."
This reverts commit dd904af566.

Reason for revert: breaks ANGLE

Original change's description:
> Improve support for arrays in Metal.
>
> Arrays in Metal now use the `array<T, N>` type instead of the C-style
> `T[N]` type. This gives them semantics much more in line with GLSL,
> so they can be initialized and assigned like GLSL arrays.
>
> This allows the ArrayTypes and Assignment tests to pass, so they have
> been added to our dm SkSL tests. (ArrayConstructors also passes, but
> is not ES2-compliant so it is not enabled.)
>
> Change-Id: Id1028311963084befd0e044e11e223af6a064dda
> Bug: skia:10761, skia:10760, skia:11022, skia:10939
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365699
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

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

Change-Id: If6a18dea7d6a45fa7836e9129bf81c2e536f07e3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10761
Bug: skia:10760
Bug: skia:11022
Bug: skia:10939
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365976
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-04 00:50:34 +00:00
John Stiles
ecd2036eae Migrate more SkSL tests to run in dm.
- CastsRoundTowardZero
- Functions
- GeometricIntrinsics
- HelloWorld
- Hex

Other test updates:
- DeadLoopVariable (for-loop structure incompatible with ES2)
- FunctionPrototype (RTE currently disallows calling undef'd functions)
- HexUnsigned (unsigned types incompatible with ES2)

Change-Id: I794bee24dfac55281c9b8b130529888ba2546290
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364678
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-03 23:45:15 +00:00
John Stiles
dd904af566 Improve support for arrays in Metal.
Arrays in Metal now use the `array<T, N>` type instead of the C-style
`T[N]` type. This gives them semantics much more in line with GLSL,
so they can be initialized and assigned like GLSL arrays.

This allows the ArrayTypes and Assignment tests to pass, so they have
been added to our dm SkSL tests. (ArrayConstructors also passes, but
is not ES2-compliant so it is not enabled.)

Change-Id: Id1028311963084befd0e044e11e223af6a064dda
Bug: skia:10761, skia:10760, skia:11022, skia:10939
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365699
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-03 22:33:19 +00:00
John Stiles
248f57b39b Reject array constructors with the wrong number of elements.
We now catch this error at IR generation time; previously we'd send it
to the driver (where it would fail to compile).

Change-Id: I45890214ffa164be1c0f359320f942bc4dc479ca
Bug: skia:11265
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365697
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-03 22:33:02 +00:00
John Stiles
56233d1379 Migrate matrix SkSL test to dm.
This uncovered a bug in Metal code generation of `matX *= matY` which is
now fixed. (It was emitting the helper function more than once.)

Change-Id: I0aeb0efe7ab5fbf5592a8ca6f4f5b50354d3d7f4
Bug: skia:11262
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365489
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-02-03 20:42:57 +00:00
John Stiles
5a956ce54b Add more dm tests for the comma operator.
Change-Id: I37a95fd95c23a91c3510f922bab0569093e612a5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364604
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-03 01:28:48 +00:00
John Stiles
d842feea48 Avoid comma usage in Operators test.
This works around a GLSL compilation bug on the Tecno Spark 3 Pro.

Change-Id: I516bd64745a8e99cccc87ee4bb2e1f5d5b26c130
Bug: skia:11255
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364116
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-02-02 14:34:24 +00:00
John Stiles
931cad1d74 Update function-type-matching tests to work as Runtime Effects.
Previously, our only test was invoking `sin(1)` which is a pretty
ineffective test. Now, we test args and return types for all the basic
scalars/vectors/matrices.

Change-Id: I7d335303eef8b9c9c6cfef2265a15bbd9bd73e0c
Bug: skia:11246
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363943
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-02 13:47:35 +00:00
John Stiles
9fc9b87540 Add bool2/bool3/bool4 to GrSLType.
These are basic vector types, required by GLSL ES2, but we could not
create helper functions using them because they were missing from our
GrSLType enum. (This also prevented Runtime Effects from using these
types in helper functions.)

Change-Id: I78c328499e8ed90cb29c641b90ee59460a5a45de
Bug: skia:11246
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364036
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-02 00:25:29 +00:00
John Stiles
c068a8f00b Update various shared/ tests to run in dm.
These tests have updated to return green on success, or red on failure.
Some tests were modified slightly to conform to ES2 limitations, or
split into separate ES2 and ES3 parts.

Change-Id: Ib47aeca217aef33f3c4b5999d93afed5d42a1e62
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363876
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-01 22:06:59 +00:00
Brian Osman
21d2b6a651 Use unsigned values when constant-folding integer vectors
We already had this trick for scalar integers, this extends it to
integer vectors. As with prior work in this area, it would be better to
detect this case and produce an error, but now we at least produce
consistent and well-defined results (rather than undefined signed
integer overflow).

Bug: skia:10932
Bug: oss-fuzz:29494
Change-Id: I45526fe96b6ea42c0e88b9862f6961b316810321
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363962
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-01 21:44:28 +00:00
Brian Osman
a7eb6819da SkSL: Do left-shift constant-folding with unsigned integers
Every GPU API defines things this way, but C++ does not (until C++20).
Forcing the shift-as-unsigned prevents UBSAN warnings.

Bug: oss-fuzz:29093
Change-Id: Ie82e25c46e28446ffc42a40172ca6b272bf1c362
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363937
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-01 19:51:28 +00:00
John Stiles
2ff8f86205 Run the SkSL Operators test as a Runtime Effect.
The test has been split into an ES2 version and ES3 version; the ES2
side omits unsupported integer ops like << >> & | ^ %.

Change-Id: Iba16d469a477809b17a823b1c68ae8937624c68e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/362616
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-01 17:21:33 +00:00
John Stiles
94e72b90b8 Add test to verify side effects from the comma operator.
SkVMCodeGenerator was lacking support for the comma operator entirely;
this has now been implemented.

Change-Id: I9350f54e6ee52764c620116e6dbfe4ca3e9cd47e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363096
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-01 16:16:58 +00:00
Mike Klein
6eee6b2736 Revert "Add intrinsic tests for mod() and fract()."
This reverts commit f06aeedcf1.

Reason for revert: fract failing on Tegra3

Original change's description:
> Add intrinsic tests for mod() and fract().
>
> These are fully supported by ES2 and will be covered by dm tests.
>
> Change-Id: Iebf4effe8ab928662b55c0bb5b09e8b2a61487ca
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/362460
> 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>

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

Change-Id: Ib2071c90addd01e3b299553e020950a89eb01e4d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/363036
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2021-01-30 13:38:31 +00:00
John Stiles
f06aeedcf1 Add intrinsic tests for mod() and fract().
These are fully supported by ES2 and will be covered by dm tests.

Change-Id: Iebf4effe8ab928662b55c0bb5b09e8b2a61487ca
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/362460
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>
2021-01-29 21:51:54 +00:00
John Stiles
9c9f60cdb4 Implement additional Runtime Effect intrinsic tests.
Tested in dm:
- max(halfN, halfN)
- max(halfN, half)
- min(halfN, halfN)
- min(halfN, half)
- clamp(halfN, halfN, halfN)
- clamp(halfN, half, half)
- mix(halfN, halfN, halfN)
- mix(halfN, halfN, half)

Compiled outputs only (not in ES2):
- max(intN, intN)
- max(intN, int)
- min(intN, intN)
- min(intN, int)
- clamp(intN, intN, intN)
- clamp(intN, int, int)
- mix(halfN, halfN, boolN)  - broken in Metal/SPIR-V (skia:11222)
- mix(intN, intN, boolN)    - broken in Metal/SPIR-V (skia:11222)

Change-Id: Iaafc5429b16d2a8710b9d171ae281c268c0fd70d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361438
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-29 18:23:48 +00:00
John Stiles
e1658b5977 Reland "Add some SkSL intrinsics to our dm tests."
This reverts commit b576168c8c.

Reason for revert: disabled floor test due to undiagnosed ANGLE + DX9 + Intel6000 failures

Original change's description:
> Revert "Add some SkSL intrinsics to our dm tests."
>
> This reverts commit 0492a744a5.
>
> Reason for revert: Intel HD6000 + ANGLE DX9 fails the floor() test.
>
> Original change's description:
> > Add some SkSL intrinsics to our dm tests.
> >
> > This CL adds dm coverage for:
> > - abs(half)
> > - sign(half)
> > - floor
> > - ceil
> >
> > And creates test output for abs(int) and sign(int); these aren't covered
> > by dm because they don't exist in ES2 and so are unsupported by Runtime
> > Effects.
> >
> > Change-Id: Ia3e660408cef50dec8fa4b6bdc12906e96179f6e
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360419
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I62121efee9315b16e61e7d38659b6f629bdf8bd8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/362056
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

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

Change-Id: I22f22f631d85d93a8fe5686a99311ec2cf85fa4d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/362103
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-29 17:58:18 +00:00
John Stiles
b576168c8c Revert "Add some SkSL intrinsics to our dm tests."
This reverts commit 0492a744a5.

Reason for revert: Intel HD6000 + ANGLE DX9 fails the floor() test.

Original change's description:
> Add some SkSL intrinsics to our dm tests.
>
> This CL adds dm coverage for:
> - abs(half)
> - sign(half)
> - floor
> - ceil
>
> And creates test output for abs(int) and sign(int); these aren't covered
> by dm because they don't exist in ES2 and so are unsupported by Runtime
> Effects.
>
> Change-Id: Ia3e660408cef50dec8fa4b6bdc12906e96179f6e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360419
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>

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

Change-Id: I62121efee9315b16e61e7d38659b6f629bdf8bd8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/362056
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-01-29 14:03:36 +00:00
John Stiles
0492a744a5 Add some SkSL intrinsics to our dm tests.
This CL adds dm coverage for:
- abs(half)
- sign(half)
- floor
- ceil

And creates test output for abs(int) and sign(int); these aren't covered
by dm because they don't exist in ES2 and so are unsupported by Runtime
Effects.

Change-Id: Ia3e660408cef50dec8fa4b6bdc12906e96179f6e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360419
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-28 21:20:00 +00:00
John Stiles
da5cdf6a70 Add default uniform binding value for Metal/SPIR-V.
This allows interface blocks in Metal to compile even if
`layout(binding=...)` is not specified. It will also be used in SPIR-V
in the followup CL, when an interface block is automatically synthesized
for top-level uniforms.

This CL also reorganizes the unit tests around uniforms a bit.

Change-Id: Ia898c536b454dda6f51677e232a8f6e6c3606022
Bug: skia:11225
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360778
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-28 19:23:49 +00:00
Brian Osman
bf7b4b8890 Add SkSL test for functions that operate on structs
This is a known deficiency of runtime effects, next step is to fix how
they manage function signatures to solve the problem.

Bug: skia:10939
Change-Id: Id934e0acdf774b03bd6edce78d7b2c077bdeae00
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360603
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-01-28 17:01:59 +00:00
Brian Osman
00fea5b41f Remove ASTNode::TypeData
Pre-cleanup as I start looking at how structs are parsed and handled in
the IR.

Bug: skia:11228
Change-Id: I6334d1073211cbbdf69ddffa8df420c45fd59fcc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361059
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-01-28 16:56:59 +00:00
John Stiles
7b92897d62 Reland "Add SkSL for-loop control flow test to dm."
This reverts commit 578f1acbe8.

Reason for revert: updated test to pass on Mac Intel 5100/6000

Original change's description:
> Revert "Add SkSL for-loop control flow test to dm."
>
> This reverts commit a0c266283a.
>
> Reason for revert: failing on Mac Intel
>
> Original change's description:
> > Add SkSL for-loop control flow test to dm.
> >
> > While loops and do-while loops remain untested in dm, as they are not
> > supported in ES2 (and therefore not available in Runtime Effects).
> >
> > Change-Id: I2f1bfccccd571cc4ced096bc18ebbb9ecc9f9b4a
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359556
> > 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>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I45335d16a695644eaeb8a535298c0efcc616c1ce
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359840
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

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

Change-Id: I2dc6e870393708a12286658001b723f25a6aec4a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359856
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-27 15:46:39 +00:00
John Stiles
4d6310ab20 Support half4 return values from main() in the SPIR-V code generator.
This creates a helper function, _entrypoint, which invokes main() and
assigns its result into sk_FragColor. We also make sure to prevent
sk_FragColor from being dead-stripped from the code during IR
generation.

At present this is useful for allowing our SkSL test shaders to compile.

Change-Id: I2d7fab0e1959a77778ffdb18ca569e869bcaeece
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358525
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-27 02:46:03 +00:00
John Stiles
b41d5bb3a7 Add helper uniforms to Runtime Effect tests.
This lets us use descriptive names like `colorRed` and `colorGreen`
instead of `half4(1,0,0,1)` and `half4(0,1,0,1)`. It also lets us use
actual unknown values instead of synthesizing sorta-kinda-unknowns by
calling sqrt.

Change-Id: I61481c33b7ff42182955777b05cfa5fcc13e0efc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359567
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-27 00:53:46 +00:00
John Stiles
d8fc95dc02 Add default uniform set value to SkSL Program settings.
This allows uniforms to be specified without an explicit `layout(set=N)`
modifier. They will assume a default set value instead.

This turns out to fix a handful of tests in Metal/SPIR-V which were
written with GLSL in mind, or adapted from real generated GLSL code, and
didn't have layout information specified on their uniforms. It will also
make it easier to write SkSL tests using uniforms that can compile
either as a runtime effect or as plain Metal/SPIR-V code.

Change-Id: Id79ec06f278b913a45c09c2e6211195dc98b42c0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359838
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-01-27 00:52:16 +00:00
John Stiles
6742d13cbe Add tests for int overloads of various intrinsics.
Checking each variation can occasionally shake out extra bugs--for
instance, the mix intrinsic in SPIR-V breaks when adding coverage for
its various overloads here. (See skia:11222; this will be addressed in
a separate CL.)

Change-Id: I2c3ca7523e59d4c6cce25a70e081a558afedfb87
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359758
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-26 22:49:20 +00:00
John Stiles
0ecfdea199 Add SkSL unit test for unbraced blocks that optimize away.
In most cases, this works properly and a `;` is emitted, but in one
particular case (int x, y;) we get nothing.

Change-Id: If88d92502f6a533284dd4e0f78daedaf1481ff3d
Bug: skia:11218
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359558
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>
2021-01-26 19:06:41 +00:00
John Stiles
e3a91cf31c Fix fuzzer-discovered error with variable scoping.
In GLSL and SkSL, control statements don't require explicit braces
around single-statement children. That is, the `match = true` child
statement here doesn't need to be braced.

    if (condition) match = true;

Because there are no braces, we never create a Block or a dedicated
SymbolTable here.  This is normally not a problem, but the fuzzer
discovered that it can dump things into the symbol table inside a child
statement:

    if (condition) int newSymbol;

This becomes problematic because the symbol name now outlives its block.
This means `newSymbol` can be referred to later, which should be illegal
(and can cause the optimizer to blow up since the structure is bogus).

There doesn't seem to be any reason to allow this code to compile; the
user can add an explicit scope here to make it reasonable, and it's
(almost) meaningless to declare a symbol that's instantly going to fall
out of scope. This code is now rejected with an error message.

Change-Id: I44778e5b59652d345b10eecd4c88efbf7d86a5e0
Bug: oss-fuzz:29849
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358960
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-26 17:08:59 +00:00
John Stiles
dac139abd9 Add unit test for control statements in for loops.
Change-Id: I94094be7163a04bf48e86406230156a5433469b6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359140
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>
2021-01-26 17:07:51 +00:00
John Stiles
7add07f88d Add unit test for control statements in while loops.
Change-Id: I924ac75b5f8a397f7af7a06925ef0c9deba5c509
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359141
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>
2021-01-26 17:07:36 +00:00
John Stiles
798021ada3 Add unit test for control statements in do-while loop.
Change-Id: I8309940f8e40d0e84847ae272830896d010c39de
Bug: skia:11219
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359138
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-26 16:20:31 +00:00
John Stiles
76389b7d24 Fix fuzzer-discovered errors in SkSL parser.
The previous change caused varDeclarations() to sometimes return an
expression-statement. This only made sense in the context of being
called from Parser::statement(). Other places which called
varDeclarations() expect vardecls and nothing else.

Change-Id: I562657cadfa20dcd77b527f2dc43dca0c6bf389f
Bug: oss-fuzz:29845
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358528
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-26 00:47:13 +00:00
John Stiles
053f785903 Avoid relying on bit-perfect irrational numbers in test code.
Comparing sqrt(5) against a variable containing sqrt(5) was not working
properly in some versions of Android running Vulkan.

Change-Id: I4f6bbff78a9ba56ec6e222f2037d66b13e3cd635
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358530
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-25 21:27:19 +00:00
John Stiles
232dd2b61b Reland "Run unit tests to verify SkSL folding behavior."
This is a reland of 4ecab92584

This reland folds in subsequent code cleanups and disables a test that
failed on Android + Vulkan.

Original change's description:
> Run unit tests to verify SkSL folding behavior.
>
> The unit test loads SkSL source files from `resources/sksl`, compiles
> the code, and uses SkRuntimeEffect to render a pixel using the effect.
> If solid green is rendered, the test passes.
>
> Change-Id: I2ccb427a907975ae84aee19d8e68d774b2cb638c
> Bug: skia:11009
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355983
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: skia:11009
Change-Id: I09196c8ca3041e8957324a0cbb7f7d6963c6e4e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358523
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-01-25 17:49:36 +00:00
Greg Daniel
95c2994048 Revert "Run unit tests to verify SkSL folding behavior."
This reverts commit 4ecab92584.

Reason for revert: breaking all the vulkan bots

Original change's description:
> Run unit tests to verify SkSL folding behavior.
>
> The unit test loads SkSL source files from `resources/sksl`, compiles
> the code, and uses SkRuntimeEffect to render a pixel using the effect.
> If solid green is rendered, the test passes.
>
> Change-Id: I2ccb427a907975ae84aee19d8e68d774b2cb638c
> Bug: skia:11009
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355983
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

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

Change-Id: Ife32f6c33d9ba7a9580b66eb312cffb249c43cb2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357780
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
2021-01-23 00:01:23 +00:00
John Stiles
97d18174d6 Allow Metal shaders to return half4 colors.
This allows us to write SkSL shaders which are valid both for use as
Runtime Effect, and for compilation with skslc targeting Metal.

Change-Id: I74e125d81865d4092e657a7d9948d2e72054bda5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357777
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-22 23:24:02 +00:00
Brian Osman
2a4c0fbdca Improve memory layout handling in SPIRV generator
Added asserts that verify we don't try to emit the same struct or array
with two different memory layout rules. Some code paths were failing to
inspect the associated variable, leading to incorrect errors about the
attached offsets of members.

Added a test case that triggered that error, and also triggers the new
asserts.

Then, fixed the underlying cause: writing out the struct definition as a
side effect of accessing a member in getLValue().

Bug: skia:11205
Change-Id: I6e5fb76ea918ec9ff10425f2d519ddbc54404b27
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357436
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-01-22 22:16:48 +00:00
John Stiles
4ecab92584 Run unit tests to verify SkSL folding behavior.
The unit test loads SkSL source files from `resources/sksl`, compiles
the code, and uses SkRuntimeEffect to render a pixel using the effect.
If solid green is rendered, the test passes.

Change-Id: I2ccb427a907975ae84aee19d8e68d774b2cb638c
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355983
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-22 20:34:28 +00:00
John Stiles
bfc9be0f77 Migrate SkSL test inputs to the resources/ directory.
This will allow us to load these inputs for unit testing in `dm`.

Change-Id: Id256ba7c30d3ec94b98048e47af44cf9efe580d5
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357282
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-22 18:57:29 +00:00