The only type of expressions that getConstantSubexpression could ever
return are Literal and nullptr. getConstantValue now returns an
optional<double>; nullopt indicates a non-constant value in the slot.
This simplifies most use cases, and allows us to get rid of some extra
"zero" and "one" Literal objects in some of our Constructor classes.
This change fixes a recent fuzzer issue. The fuzzer had discovered that
calling `getConstantSubexpression` on a ConstructorCompoundCast that
contained a compile-time-constant value would return literals of the
wrong type (the cast was not applied). By nesting repeated matrix casts,
this type confusion could be turned into an assertion.
Change-Id: Icee69219e6db2822ffdfab4e5ccdaff54584a4b6
Bug: oss-fuzz:41000
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471376
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Yesterday's implementation was close but I realized later that it wasn't
quite ideal.
- Array index-folding was gated on `isCompileTimeConstant`, which is too
strict. The real limitation is `hasSideEffects`. If an array contains
a side-effecting expression, we should leave it alone. Otherwise it
is safe to pluck out an element from the array and toss the rest.
- Matrix index-folding was gated on `getConstantSubexpression` for the
extracted elements, but did not check the other elements at all. This
was too lenient; we now only proceed to the folding step if
`hasSideEffects` returns false.
I added some tests to verify the final behavior and also discovered a
small related issue. Diagonal matrices were not substituting literals
in for constant-values, which inhibited folding as well and would break
constant-expression evaluation. This is now fixed.
Change-Id: Idda32fd8643c1f32ba21475251cd4d4dd7cea94c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470396
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Indexing into a constant matrix is a constant expression, so we are
obligated to support it for ES2 compatibility.
Change-Id: Ibe1e5bac39d9a88ce0222997a38e8b6952fdb336
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469819
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We should support constant-expressions involving matrices (GLSL ES2
does, WebGL does). We currently don't. We do properly report out-of-
range indexing, but we don't optimize away valid matrix index
expressions or allow matrices to be indexed in a constant-expression
context.
Change-Id: If58aa4c5f15abef421a412957072f3617b4176df
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469818
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, SkSL was unable to resolve the constant expression `x[y]`
for a constant-array `x` and a constant-integer-scalar `y`. Now, if `x`
and `y` are known, we can replace `x[y]` with the indexed array element.
Note that we need to be careful here, as it's not a valid optimization
to eliminate array elements that have side effects. We preserve side-
effecting expressions using the comma operator.
Change-Id: I5721337eb42b48c0b05f919c1cadfae19dd3b84f
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469839
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, we didn't have tests which leveraged constant-evaluation of
array indexing (because we didn't support it), and our test files
commingled constant-indexing into vectors with constant-indexing into
arrays.
The test files now separate vector- and array-handling into separate
tests, and a ton of new cases have been added to ArrayFolding. The
ArrayFolding tests now require constant-evaluation of array indexing,
so they fail in this CL, but will be fixed in the followup CL.
Change-Id: I3b663e743d97d6db80627bc9b7808f88c99917a7
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469528
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
See http://review.skia.org/460037 for an example of the existing
behavior. Const variables are constant-expressions and should be allowed
here.
Change-Id: I41383d79668785f270b7825485e9f6fa56c553c1
Bug: skia:12549
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460036
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This fixes GLSL ES2 conformance test `array`.
Change-Id: I6ebee9253e1e8c394d9ddb6899e3a0940b7a38ef
Bug: skia:12495
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458718
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
If x is a known compile-time constant value, it can already be optimized
to a final value.
If x is not known, it could be zero, and 0/0 should result in a NaN.
Change-Id: I643a7c6da0a43ec366235c4df39fc78d3b361de7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441580
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>
The additional tests from http://review.skia.org/441238 uncovered a gap
in the constant folder's abilities; it was not able to fold away
boolean vector comparisons even when they were constant. These are ES2
constant-expressions, so folding them properly is a requirement.
Change-Id: Ia0b4d5d1215c5fc2b247ac3f0dec4c8747d2153e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441579
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>
The no-op-arithmetic simplifier was written before we allowed casting a
mat2x2 to a float4, and did not expect a matrix inside a vector ctor.
The expression `float4(myMat2) * float4(anything)` would assert when we
tried to determine if `myMat2` was a constant zero or constant one.
The code has been rewritten to use getConstantSubexpression and now
allows matrices inside.
Change-Id: Id625141256bf89d816c57d2d21f16b0ec252c158
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440858
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
GLSL disallows mixing swizzle domains within a single swizzle:
http://screen/93eHNQDbx35hMdk
SkSL now disallows it as well.
Change-Id: Ied2e11ee04285b143a864e28cac30335f01aad0e
Bug: skia:10621
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426458
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
In the majority of cases, a uniform is an equally good substitute, and
replacing `sqrt(N)` with `unknownInput` actually makes the test clearer.
Change-Id: I7bcb477571972d7aa2ce8c49b3674471f7310748
Bug: skia:12034
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/411306
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is similar to the intrinsic optimization for any() and all(). Tests
for all three intrinsics have been bulked up a bit as well.
Change-Id: I262b9448e543b4709d1e7c8585f74a206c4b5abd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/406576
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
A cast like `float(five)` or `int4(colorGreen)` should detect const
variables and replace the expression with its compile-time constant
equivalent value. At present, this replacement is missed, which inhibits
further optimization opportunities on the expression.
(This CL is very similar in spirit to http://review.skia.org/404676)
Change-Id: I04b5c435a30d2afcdbdb3d020adc15e9c651cc31
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405682
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, the code neglected to resolve constant variables into
values. This meant that expressions like `lessThan(zero, one)` could not
be compile-time evaluated even when `zero` and `one` have known values.
Change-Id: I2f5ce303e3dcc682be14e4d2485e24dd7c59212e
Bug: skia:10835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405536
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
If every argument passed to any() or all() is a compile-time constant,
we now detect this and optimize away the function call entirely.
Future CLs will perform a similar optimization on other intrinsic calls
which can be detected and eliminated at compile time, like lessThan().
Change-Id: Ie55aff538b1ccaf2b3bcf9a69573a85f081b7ade
Bug: skia:10835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404417
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
