We had all the moving parts in place, but had neglected to check
`simplify_constant_equality` for structs. It worked for structs that
boiled down to the same expression tree, but not for mismatched structs.
Comparison of known structs should be allowed in a constant-expression.
Change-Id: I3a52abf4530d1fe25d17eb482c1e29b5b8e03cf6
Bug: skia:13387
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547444
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This is necessary for proper constant-folding, and can help us generate
better code. Most examples are pretty contrived, but a realistic benefit
can be found in inlined code (see TrivialArgumentsInlineDirectly.glsl).
Change-Id: I86aac9d48a70ae35a15109fd0b04400ae39a3b47
Bug: skia:13387
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547277
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 3ba37e1d38.
Reason for revert: disabled test on Win+Intel/Android+Adreno6xx GPUs
(skia:13393)
Broke out non-folding tests to a separate test due to failures on
Quadro/RTX3060. (skia:13395)
Original change's description:
> Revert "Add unit test covering struct field folding."
>
> This reverts commit 0cbba91940.
>
> Reason for revert: returns red on Adreno and Intel
>
> Original change's description:
> > Add unit test covering struct field folding.
> >
> > At present, we don't try to optimize away `myStruct.myField` accesses
> > even when `myStruct` is known or constant, so the output from the test
> > is not too impressive.
> >
> > Change-Id: I563559e5cdc6c2669d69ec78ad8ca09d3be02a68
> > Bug: skia:13387
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547276
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> Bug: skia:13387
> Change-Id: I2e651ddb82fac08cdc16fa8b77696cdd314e805f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547438
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Change-Id: Ic86ff6b1034363d1343793f94e3ba707adb2fcc3
Bug: skia:13387, skia:13393, skia:13395
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547439
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 0cbba91940.
Reason for revert: returns red on Adreno and Intel
Original change's description:
> Add unit test covering struct field folding.
>
> At present, we don't try to optimize away `myStruct.myField` accesses
> even when `myStruct` is known or constant, so the output from the test
> is not too impressive.
>
> Change-Id: I563559e5cdc6c2669d69ec78ad8ca09d3be02a68
> Bug: skia:13387
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547276
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:13387
Change-Id: I2e651ddb82fac08cdc16fa8b77696cdd314e805f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547438
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
At present, we don't try to optimize away `myStruct.myField` accesses
even when `myStruct` is known or constant, so the output from the test
is not too impressive.
Change-Id: I563559e5cdc6c2669d69ec78ad8ca09d3be02a68
Bug: skia:13387
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547276
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
"canUseAnyFunction" was totally unused. All of the others that I removed
are only used from C++ code to control higher level logic (not within
shaders). A few of the remainders don't have sk_Caps references today,
but adding usage seems plausible.
Change-Id: I196f7d8abacde9dc6903d792cd18b58a34dc19f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546858
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This lets us test non-constant array indexing and array ctors (which ES2
doesn't allow at all).
I also added some cast-related tests to the non-ES3 test which were
accidentally missed the first time around.
Change-Id: Ib03aa53f57702141a8852154d584a31ab75f96e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547017
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Test cases were added for matrix-resize-ctor, diagonal-matrix-ctor and
struct-ctor. These were originally absent because the original trivial-
expression code did not distinguish between each individual ctor type.
Unfortunately, array-ctor is not included because ES2 does not support
array-ctors.
Comments were added to make it a bit easier to understand why some cases
are trivial but others are not. In some cases, the compiler applies
relevant transformations that might not be apparent at first glance--
e.g. unary + is eliminated so that case is still trivial.
Change-Id: Ie6756fc01736d28035f3965ae18f3482bd030871
Bug: skia:13378
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546864
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 5270322b46.
Change-Id: If594d04dc657126dce48d69dcc67d1a5e3b0cc8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546856
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This code was originally written before our constructor IR types were
split apart, so its treatment of constructors was pretty simplistic. The
new code handles each type of constructor separately, and makes better
decisions as a result.
Compound constructors are now only considered to be trivial when they
are compile-time constants; this causes vector<->matrix conversion
constructors to be detected as non-trivial.
Change-Id: I534fcf69f5d5f43ac705d68ae00f97fce8496c2a
Bug: skia:13378
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546555
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
GrSPIRVUniformHandler and GrVkUniformHandler had special-case handling
of float2x2 matrices, but neglected to put in the same handling for
half2x2 matrices (which are identical at the uniform level).
There are likely to be deeper issues here, but this is a step in the
right direction compared to where we were.
Bug: skia:13380
Change-Id: I035f707d1f3831c0057934fe16b2f96ea4847578
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546550
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This version dodges skia:13380 by reordering the list of uniforms.
Disabled on Win10 with older Intel GPUs due to crashes when performing
vector/matrix casts in GLSL.
Change-Id: If053908fd1a6921257e5af0abdce1ff69a03297f
Bug: skia:13378, skia:12179
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546551
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 9583759bbd.
Reason for revert: MSAN failure
Original change's description:
> Restored unsized array support to SkSL
>
> This is a prerequisite for compute shaders. As of this CL, there isn't
> yet a way to use unsized arrays, as it is a compute-only feature and
> compute shaders are coming in a followup CL, but this adds the basic
> framework and error tests.
>
> Change-Id: I390c0961e324dd474474563bf9a8f6b34c9552a9
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538900
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Id10b48ef24c0e6219b65b0a201d13fea9632620f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546552
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This reverts commit c18fb9c8a2.
Reason for revert: Red on the tree.
Original change's description:
> Add test demonstrating that matrix/vector casts are considered trivial.
>
> These casts are not particularly lightweight Metal (they invoke a helper
> function) so they are going to be marked as non-trivial in a followup
> CL.
>
> Change-Id: Ic88e6b3889f6d4f4751d7ebfc740229b7530930b
> Bug: skia:13378, skia:13380
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546417
> Reviewed-by: Arman Uguray <armansito@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:13378, skia:13380
Change-Id: Iee60fcc0d9d56912268777a9fa4d2aa18154f157
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546548
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
These casts are not particularly lightweight Metal (they invoke a helper
function) so they are going to be marked as non-trivial in a followup
CL.
Change-Id: Ic88e6b3889f6d4f4751d7ebfc740229b7530930b
Bug: skia:13378, skia:13380
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546417
Reviewed-by: Arman Uguray <armansito@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This is a prerequisite for compute shaders. As of this CL, there isn't
yet a way to use unsized arrays, as it is a compute-only feature and
compute shaders are coming in a followup CL, but this adds the basic
framework and error tests.
Change-Id: I390c0961e324dd474474563bf9a8f6b34c9552a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538900
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
DSLType stored an `fPosition` which was only used in one place, when
reporting that a function is not allowed to return a particular type.
Those errors now highlight the type and function name together, which is
not really any worse than before. This allows us to shrink DSLType down
to its minimal form, just a pointer to an SkSL::Type and nothing else.
Change-Id: I4b430cb996472da0ae57bc2ab095cd123d2c3f51
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546097
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Booleans and structs/interface blocks that transitively contain a
boolean member are no longer allowed to be used as a uniform. This is
because SPIR-V and WGSL currently disallow OpTypeBool in host-shareable
storage classes.
Change-Id: I10315c7f261ff10a07636265968a91d9c421da55
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542776
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
* Added support for field access expressions, excluding user-defined
interface blocks.
* Added a test case for vertex stage builtin declarations.
* Added a NOP workaround for sk_PointSize which does not have a
WGSL/WebGPU equivalent.
Bug: skia:13092
Change-Id: I46fd7bb0a9fe55d2e3ed2b3b72f63376c1ed4f31
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539082
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
- Add support for simple assignment expressions, as well as swizzle,
variable references, and type conversion constructors to support
simple assignment test cases that reference built-ins and local variables.
- Handle a case where the type of a SkSL built-in differs from its WGSL
counterpart and emit a type cast when such a variable gets referenced.
- Add additional test cases for supported WGSL features that could not
be tested without simple assignment support.
Bug: skia:13092
Change-Id: Ib1ff3bcef60e436c9be5c08236c9fe4de02dd005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538420
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
WGSL requires that pipeline stage IO parameters that are scalar or
vector integers be annotated with the "flat" interpolation attribute.
Bug: skia:13092
Change-Id: I2930da6cf468ac8cd9ccaca0004e5849065411dd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538419
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
1. It is illegal to declare an empty struct in WGSL. The codegen no longer
declares an empty struct for pipeline stage inputs and outputs if the
program does not reference them.
2. WGSL requires that every vertex program declare a @builtin(position) output.
The codegen now emits this pipeline stage output even for an empty
program that doesn't assign to sk_Position. This allows unit tests for
vertex programs to be authored without unnecessary boilerplate.
Bug: skia:13092
Change-Id: Ib023200e2ba13acf96859817249686d7f815cb00
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538418
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
- Introduced the SK_ENABLE_WGSL_VALIDATION macro which is currently only
enabled when skslc gets compiled when using the `skia_compile_sksl_tests`
setting.
- SkSLCompiler::toWGSL now validates its output using Tint's WGSL reader
structures based on conditionally compiled code depending on the
SK_ENABLE_WGSL_VALIDATION flag.
- Fixed `warning: use of deprecated language feature: struct members should be separated with commas"
warnings that were generated for HelloWorld.wgsl.
Bug: skia:13092
Change-Id: Ib894457030004966221faf82f61360e390b95e22
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537802
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
ProgramSettings now has a fRequiredVersion, based on the #version
directive (if any). There is plenty more to do, and this is an
intermediate state where the "fEnforceES2Restrictions" bool still
exists. Eventually, that will go away, or change into a check between
the required version and the SkCapabilities supplied (also in
ProgramSettings).
As a proof-of-concept, this migrates some use-cases to insert the new
version directive, and unlock ES3 features in some tests and benches.
Bug: skia:11209
Change-Id: I964975931039588d7a302bfedd09246e3d4a79d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540037
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
OpenGL docs specifically insist that the sequence (comma) operator
should not be treated as a constant-expression so that attempts to
declare multidimensional arrays with a comma will fail:
http://screen/vJEpAe9yNmbzZTm
(See "12.43 Sequence operator and constant expressions" in the OpenGL
ES3 documentation or read skia:13311 for details.)
In practice, we don't get much benefit from optimizing away unused
comma-expressions; it improves some synthetic tests, but realistically
this will not help Skia in any real-world scenario. The constant folder
no longer attempts this optimization, and comma-expressions are now
rejected in a constant-expression context.
Change-Id: Ic5dea6ff90e36614b548c1ce89a444e81da944ae
Bug: skia:13311
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539565
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We will continue to fold expressions like `10 + 0` or `5 * 1` or
`PI * 1` (assuming PI is a const float) because both sides are known,
but non-constant-expressions like `x + 0` (assuming x is not a constant)
or `foo *= 1` will be left as-is when the optimizer is off.
This improves the accuracy of the Viewer shader tab, because a runtime
effect expression like `color *= scale` will be preserved in the final
output when optimization is off, instead of being replaced with a Nop
when scale equals one.
Change-Id: I218b327cb0cd12654dca446dee8a5baa96f589b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539197
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Fixes a minor issue discovered by http://review.skia.org/539198 .
Change-Id: I63f555cc005df33ce50c412796a8c773a501d271
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539199
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously we didn't have any mechanism for disabling the optimizer when
building golden outputs, so every golden output always had optimizations
applied.
Change-Id: I8f370b06daab6cb50bb4339eab0d39578621413b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539198
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This node was only used to detect recursion while inlining. We no longer
need to do this, because we disallow recursion in all programs.
The removal of one IRNode per inlined function actually allows for
slightly more aggressive inlining, since we restrict inlining based on
IRNode consumption. This allows the "ExponentialGrowth" tests to inline
a bit more deeply than before.
Change-Id: I894dbb1ca3096bb785b67facb01cc9c630f694c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/534780
Reviewed-by: Arman Uguray <armansito@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We enforce no-recursion in all programs now, not just Runtime Effects.
Change-Id: I3737329e4526fa1b7fdbb47ccb959f78f507f665
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535119
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This limits our error reporting to the first 16MB of SkSL code in a
program, and error marks are limited to a run of 255 characters or
less. In practice, these limits do not affect normal code in any way.
This gives us the same tight memory footprint we originally had when
positions were stored as `int32 fLine`.
Change-Id: Idef04344324870a7b92aca154feb5e1a0121d284
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/533699
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These were not being generated because they were in the "settings" test
group; we have switch-rewrite settings that also need to be tested.
This was a blind spot in our golden output coverage; without these
tests, there is very little switch-statement usage in our corpus.
They are now in the SPIR-V test group as well.
Change-Id: Ic23b726d00c3047f2d19f7f6dc41e58e600e991c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/534141
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We now have a new type of ProgramKind, private runtime shaders.
`sksl_rt_effect.sksl` is now only loaded for these kinds of program.
Rather than having a special-case check for sk_FragCoord in
SkRuntimeEffect, the symbol will no longer exist at all unless a private
options flag is set.
Change-Id: I9223baaf59d74c44d64f322cd57fc841625342b7
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532784
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Right now nothing prevents it from working. (skslc doesn't use
SkRuntimeEffect::MakeInternal.) This will be fixed in a followup.
Change-Id: Ib8479220e1f194b035516d976a7369d926a07f5d
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532783
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This has two benefits:
1) We get detailed error messages with accurate positions
2) We can actually test these in our golden .rts files.
Thanks to #2, add a new unit test file, and adjust some existing files
that were breaking these rules.
Change-Id: I0b65e2f06f79ce8cbea9bad4c3d27062ec9b6e6c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532769
Reviewed-by: Arman Uguray <armansito@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:13169
Change-Id: Icf0b720d3e3a13d490aba8495cf9db83d1d62318
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532762
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This will allow us to reuse this logic in Graphite.
Change-Id: I649dcd3893a1355af457a2583a6db3066fb87c9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532758
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:13170
Change-Id: I11ef0ea5ac3ae61b24a47805bb3290a37880cfee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532536
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:13173
Change-Id: Ifbcce77605dd781563568293fc501dfa31f143da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528706
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:13171
Change-Id: I6dffb98ac2464f930995cf8ea57e422091d20fd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/531743
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
(This mirrors an optimization performed in the constant folder.)
Expressions like `OpIEqual %20 %20` or `OpFUnordNotEqual %15 %15` can be
replaced by `true` or `false` on sight. The GLSL spec makes it clear
that checking for NaN is optional:
4.7.1 Range and Precision
"... NaNs are not required to be generated. Support for signaling NaNs
is not required and exceptions are never raised. Operations and built-in
functions that operate on a NaN are not required to return a NaN as the
result."
Change-Id: I2e29b659a73582e9ade0eb61f70f7d362a007c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/531550
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously, every expression/statement type was responsible for
pruning or clearing the store-cache when branches were involved. This
was difficult to reason about and easy to get wrong, particularly if
the details are not fresh in your mind.
Now, `writeLabel` takes care of the details for you. Pass in the
location of the branch(es) which use the label, and the proper cache
updating behavior will occur automatically.
Some of the label enum types are not strictly necessary and exist for
the benefit of a reader. Specifically:
- `kBranchlessBlock` and `kBranchIsOnPreviousLine` are synonyms
- `kBranchIsBelow` and `kBranchesOnBothSides` are also synonyms
The hope is that extra enum names will be easier for a reader to
follow, versus fewer but very-verbose enum names (like
`kBranchIsBelowOrOnBothSides`).
This change earned some very minor switch-related dividends. Previously,
every label in a switch was treated as a forward-branch, but in fact,
the very first label in a switch is privileged. This is because we are
branching from the previous line, and the store cache is trustworthy in
this case. (Versus "branching from above," where the store cache needs
to be pruned before it can be trusted.)
Change-Id: I38b539069c22be9f0777b632f60f0eab2409d687
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/531540
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We now have two functions `writeOpLoad` and `writeOpStore` which are
in charge of writing SpvOpLoad and SpvOpStore instructions.
`writeOpStore` also keeps track of pointer stores in a "store cache."
Subsequent loads from that same pointer will be found in the cache and
will return the value stored in that pointer instead.
Such a cache definitely cannot work in the face of control flow, so we
make the following concessions:
- `pruneReachableOps` is now `pruneConditionalOps`. Any pointers that
are altered inside a potentially-unreachable block are cleared from
the cache entirely.
- The entire store cache is cleared at all OpLabels within a loop.
The cache also cannot work in the presence of swizzled stores, so we
make another significant concession:
- The entire store cache is cleared whenever we store into a non-memory
pointer (e.g., assigning into a swizzled LValue, such as `foo.xz`).
Despite these significant limitations, this manages to dramatically
shrink many real-world examples.
Change-Id: I0981a0cf7b45b064e153e9ada271494c8e00cad5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/530054
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, we would determine the operandType by calling
`getActualType`. This function converts half-precision types to full-
precision ones, which seems to have been unintentional. Fortunately, the
operand type is not actually emitted into the SPIR-V by most code paths
(most paths use the resultType instead) so it was not a significant
impact in practice. A few matrix-based paths emitted ops using this type
and these paths now emit RelaxedPrecision as expected.
Change-Id: I32f4c0327427476fee6b78953284818b7970b6e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/530543
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Previously, we only handled the simple case of extracting from an
OpConstantComposite. Now we also handle the complex case of extracting
from an OpCompositeConstruct, where vectors can be composed of other
vectors.
This is particularly challenging because OpCompositeConstruct can
contain SpvIds from almost any other instruction, so we need to be
able to decode those instructions and figure out their type. For
instance:
%5 = OpFAdd Vec2 %1 %2
%6 = OpFAdd Scalar %3 %4
%7 = OpCompositeConstruct FloatVec3 %5 %6
%8 = OpCompositeExtract %7 2
The %8 (OpCompositeExtract) could be replaced with %6 but we need to
peek at the type in *both* OpFAdd instructions to decode this. It
only works when the affected instructions are in-cache, so many
opportunities are currently not optimized because their code still
uses the original, uncached form of writeInstruction.
Change-Id: I5719ae6284f32e1d6f2c898eca282c22b94fc764
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529743
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:13219
Change-Id: I57c5c2aa40e6eb85d5e6045d6f3374d0379efd39
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/530337
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This removes the assert from the SPIR-V generator so the
tests compile. The generated SPIR-V is incorrect. The next
CL fixes the generator, and restores the assert.
Change-Id: I77b507cf7fb5eac481322887000bd1c73cd5c899
Bug: skia:13219
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/530336
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is a reland of commit 60ff0facbf
Structs are now deduplicated using a [Type*, SpvId] map.
Original change's description:
> Use op cache when emitting types.
>
> We no longer need to maintain a separate `fTypeMap` for mapping types
> to SpvIds, since the op cache handles this automatically.
>
> We also now support deduplicating instructions that don't have a result,
> such as decorations. (In particular, we needed to avoid emitting the
> SpvDecorationArrayStride op every time the array type was accessed, but
> this op doesn't have a result ID.)
>
> Change-Id: I779b8c8e3de5973b8f487b28c0a8ece9a1041845
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529732
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I9f6a78d58e8af38a1fd690a8860d8b5aa3193be6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529748
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I800fa2a1fb0e64ad478c76ea2d5cda176ea8f48b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529746
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit 60ff0facbf.
Reason for revert: Broke D3D bots
Original change's description:
> Use op cache when emitting types.
>
> We no longer need to maintain a separate `fTypeMap` for mapping types
> to SpvIds, since the op cache handles this automatically.
>
> We also now support deduplicating instructions that don't have a result,
> such as decorations. (In particular, we needed to avoid emitting the
> SpvDecorationArrayStride op every time the array type was accessed, but
> this op doesn't have a result ID.)
>
> Change-Id: I779b8c8e3de5973b8f487b28c0a8ece9a1041845
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529732
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I0e2187f88f2a945fd6f88ce75ff815e03d2f7df5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529747
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
