ES2 disallows opaque types in expressions (other than passing them to
their associated builtin functions). We now enforce a similar
restriction on SkSL opaque types.
While I was there, I added several other cases to the invalid-shader
test to make sure that they were all caught.
I needed to reorder some code to make sure that ternary expression error
messages didn't change. Ternary expressions now check for opaque types
before checking that the left-side type and right-side type are
compatible. This is because we check for "compatible" ternary
expressions by checking if `leftSide == rightSide` would be accepted.
`shader1 == shader2` used to be considered a valid expression for the
purposes of this test, but not anymore.
Change-Id: I62a0a31feca9dadd428da7d1b48d7693c4b6434d
Bug: skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516802
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>
The fuzzer discovered that we allow == on void types (confusing the SkVM
backend).
Change-Id: Ia9494642faf67f3f86e3a365807be8bd4a7062e4
Bug: skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516796
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, we would take the vector-folding path for all types. This
didn't cause any problem for scalars, but failed for "zero-size" types
like void. It isn't valid to compare zero-size values, but we currently
don't reject such code (see skia:13026), and the fuzzer noticed this.
It's safest to only run the vector-folding code when we actually have
multiple slots that need to be folded into one result.
Change-Id: I0bc88043d9a4aeea38ae24dc1a6d1a7430d3d7b0
Bug: oss-fuzz:45279, skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516676
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The slot-assignment logic has been changed to associate slots with
function calls, instead of function definitions. In our test case, you
can now see that the calls to `get` are now mapped to $15, $17 and $18.
This change also jiggles some existing tests and improves their
register allocation slightly (!).
One minor hitch here is that there's no FunctionCall node associated
with main() (it's never explicitly called). However, our slot map key
can be any IRNode, and we know main() can't be called by anyone else,
so it's harmless to use the function definition as the key in this case.
(This entry could probably stay out the map entirely if it made a
difference, but I don't think it matters.)
Change-Id: I68a6ff24cbd3a2db77f24126502bd3d11e8c0962
Bug: skia:13011
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514578
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
If a function is called multiple times on one line, stepping over that
line does not show all of the function-call results. It only shows the
last result.
e.g. in this example, I have just stepped over the first line which
calls "get" three times. We should see three results, but we only see
one: http://screen/3WfJoZWm77cSexM
In this test you can see that all three calls to `get` are assigned to
the same slot, $15.
Change-Id: Id0c486ef349a1e527001efbcee2ed2b836f56e83
Bug: skia:13011
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514577
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I added a comment and didn't rebuild; this shifted line numbers around,
which is reflected in the debug trace opcodes.
Change-Id: I1b8e00ff65557a03483e8d7ff0c4dbec9852866f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514777
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
The `break_loop` test causes LLVM to get confused and crash when
compiled on some GPUs. The crash goes away if we pass a literal 5
instead of a 5 that is computed at runtime. This also results in a
simpler test for SkVM, for better or worse, but we still have
coverage for dynamic loop exits in other tests.
LLVM crash: https://paste.googleplex.com/4718583155261440
Dangerous shader: https://paste.googleplex.com/4776089520963584
Change-Id: Ic6cbd55a36d2de58e5dd3459d4dfd74acdbc9f91
Bug: skia:13005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514538
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
MSL does not support the unary "-" operator on matrix types. Similarly
the SPIR-V OpFNegate/OpSNegate operations only work on scalar and vector
type.
* An expression such as "-<mat>" is now transformed to "-1.0 * <mat>" when
generating MSL.
* The same expression now generates a component-wise negation in SPIR-V,
matching what glslang outputs for GLSL.
* A unary "+" is now treated as NOP for MSL, matching the SPIR-V backend.
An expression such as "+<expr>" is now evaluated as "<expr>".
* The shared/Negation.sksl has been moved to folding/ as much of its
contents exercise constant-folding of comparison expressions.
* The shared/UnaryPositiveNegative.sksl test has been extended to
exercise scalar and matrix types.
NOTE: The SPIR-V backend changes have caused a minor re-ordering of SSA
IDs generated when writing out a prefix-expression. The affected gold
files have been updated.
Bug: skia:12627, skia:12992
Change-Id: Iec5cdafc591aed7e49b3b52bda42a02661380bab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513976
Auto-Submit: Arman Uguray <armansito@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
We previously had no way to signal a parse error from arraySize,
resulting in a cascade of additional errors downstream. This tightens
up the behavior and allows us to fail more gracefully.
Bug: skia:12416
Change-Id: I83d3d5bc1dc63395edb325297375a6eb52415817
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512952
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
On modern hardware, this will give the correct result for `NaN != x`
(true).
Change-Id: I9683f74756da5da5f34ccacec02c1f2449791f26
Bug: skia:12977
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513317
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
I wasn't able to find any other test which exercised child color-filters
or child blenders. (SampleWithExplicitCoord evaluates from a shader.)
Change-Id: I58ecee3beca2d3dc11ded5de0eea031e1d7c3e1e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507922
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
These all stemmed from the same root cause, but are interesting and
distinct enough to include in our error tests.
Bug: oss-fuzz:44555, oss-fuzz:44557, oss-fuzz:44559, oss-fuzz:44561, oss-fuzz:44565
Change-Id: I22c1798809754b4b38c77ffbe369a97c64a2f60e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The fuzzer constructs a long, valid nonsense expression
(x+x+x-x+x-x, etc.) which exceeds parse depth. At that point, the token
stream points to a `+` token. The parser attempts to consume a new
statement but stops in `unaryExpression`; this fails again, due to the
max parse-depth, but doesn't consume a token. The parser continues
trying to parse the statement, but stopping in `unaryExpression`, making
no forward progress in an infinite loop.
I've made a couple of changes as a result.
- Exceeding the max parse depth now sets `fEncounteredFatalError`.
- Encountering a fatal error causes block() to immediately halt. This
actually undoes a few of the arbitrary changes from
http://review.skia.org/506463 but not in a bad way.
- `unaryExpression()` now consumes a token before checking parse-depth.
- `structDeclaration()` had a similar issue where it could potentially
fail without consuming any tokens; this is fixed as well.
- Some unnecessarily-nested logic in ternaryExpression() was flattened
while I tried to ensure that it always consumes a token.
Change-Id: I52c2161965ffbcef1185761ca6897ec1cba5df89
Bug: oss-fuzz:44551
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507436
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This enables the SkSL error testing logic for runtime effects. The core
logic is identical, only the ProgramKind differs.
(Error creation scripts: http://go/paste/6413797460803584 with some
light post-processing)
Change-Id: I877205b3cc1014b50ccccf6037a2f4034c07543e
Bug: skia:12665
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506538
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, when the parser found a bad statement inside a Block, it
would stop processing that Block entirely. This caused our brace
matching to fall out of balance. block() would normally only return once
the Block's closing brace was consumed, but in this case, the closing
brace would still be in the parse stream awaiting consumption even
though block() had returned.
Now, when a bad statement is found inside a Block, we just ignore it and
continue processing. (I tried injecting a poisoned statement as well,
to see if it would affect the test results, but they were identical.)
This seems to generate somewhat better errors.
Change-Id: I8dc781d5602bf99d7610f8280cde8b7c1925cb65
Bug: skia:12868
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506463
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
std::stringstream has a subtle bug in OS X 10.12. Reading in a too-large
floating point value returns INFINITY but does not set failbit. This
caused SkSL to report a different error message than expected
("floating point value is infinite" instead of "floating-point value
is too large: NNNNN"). We now guard against this case in SkSL::stod by
adding an explicit `isfinite` check.
Bug: skia:12928
Change-Id: I9996e64b69512ea5710e6fc3ff00ad1ad83c247b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505939
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>
This breaks on OS X 10.12: http://screen/7A9bumDr8Z4ihcy
Debugging is difficult via a trybot. This CL can be reverted once the
root cause is discovered and fixed.
Change-Id: Ibbfadc9fbe39eb8d1755e6f382b806d1d648a6fe
Bug: skia:12928
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505803
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We no longer enforce a particular string form of 3.41e+38.
Change-Id: I33b8a30aa3c7ab54de0c7f4a02181b60cd8f71a3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505799
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This was (crudely) automated with shell scripts:
http://go/paste/5484300603490304
Change-Id: Ic9e1c93112772d303d1158eb26d995f27b439eba
Bug: skia:12665
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505637
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 43539c22a2.
Reason for revert: UB fixed at http://review.skia.org/505678
Original change's description:
> Revert "Verify that tests in errors/ actually generate the expected errors."
>
> This reverts commit 8d646c127a.
>
> Reason for revert: triggering UBSAN
> http://screen/887FeQtZWs2A6oo
>
> Original change's description:
> > Verify that tests in errors/ actually generate the expected errors.
> >
> > Error expectations are embedded in the source with a special *%%*
> > marker, like this:
> >
> > /*%%*
> > expected 'foo', but found 'bar'
> > 'baz' is not a valid identifier
> > *%%*/
> >
> > This unit test compiles every effect in errors/ and verifies that it
> > makes an error. It also verifies that the errors returned include the
> > expectations from the *%%* marker section, in the listed order, if any
> > expectations have been listed. (Error expectations are not meant to be
> > exhaustive; additional errors are allowed.)
> >
> > In this CL, I've manually attached error expectations to the first few
> > error tests. A followup CL will (mechanically) add expectations to every
> > error test, based on their current error reports.
> >
> > Change-Id: I4add30fef6419c4d3f8d2a221c5aeb53eee35ae7
> > Bug: skia:12665
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505399
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
>
> Bug: skia:12665
> Change-Id: I3bcdbe9fc1abab13656d6462b73f6439967fd96f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505642
> 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>
Bug: skia:12665
Change-Id: I49e23869f4ef383a0b076006e319e0a6d7191cad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505643
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
It's undefined behavior to cast a double to an int64 if the double is
out of range. Our SkSL error tests managed to trigger UBSAN on the tree,
pinpointing the issue (which we had already written up a bug for).
Change-Id: Ia06896732223ff310f2c175efcbeb96ba5786fa8
Bug: skia:12863
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505678
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 8d646c127a.
Reason for revert: triggering UBSAN
http://screen/887FeQtZWs2A6oo
Original change's description:
> Verify that tests in errors/ actually generate the expected errors.
>
> Error expectations are embedded in the source with a special *%%*
> marker, like this:
>
> /*%%*
> expected 'foo', but found 'bar'
> 'baz' is not a valid identifier
> *%%*/
>
> This unit test compiles every effect in errors/ and verifies that it
> makes an error. It also verifies that the errors returned include the
> expectations from the *%%* marker section, in the listed order, if any
> expectations have been listed. (Error expectations are not meant to be
> exhaustive; additional errors are allowed.)
>
> In this CL, I've manually attached error expectations to the first few
> error tests. A followup CL will (mechanically) add expectations to every
> error test, based on their current error reports.
>
> Change-Id: I4add30fef6419c4d3f8d2a221c5aeb53eee35ae7
> Bug: skia:12665
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505399
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12665
Change-Id: I3bcdbe9fc1abab13656d6462b73f6439967fd96f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505642
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>
Error expectations are embedded in the source with a special *%%*
marker, like this:
/*%%*
expected 'foo', but found 'bar'
'baz' is not a valid identifier
*%%*/
This unit test compiles every effect in errors/ and verifies that it
makes an error. It also verifies that the errors returned include the
expectations from the *%%* marker section, in the listed order, if any
expectations have been listed. (Error expectations are not meant to be
exhaustive; additional errors are allowed.)
In this CL, I've manually attached error expectations to the first few
error tests. A followup CL will (mechanically) add expectations to every
error test, based on their current error reports.
Change-Id: I4add30fef6419c4d3f8d2a221c5aeb53eee35ae7
Bug: skia:12665
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505399
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This code should be easily adaptable to matrix-times-vector as well;
just treat the vector as a 1-row or 1-column matrix. I haven't gotten
around to writing tests for this, though.
Change-Id: If59ae52cd12952b44d3574d54398b2dc66edbcc8
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505221
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>
These tests only generate an error in the SPIR-V or GLSL backends. We
will soon enforce that everything in errors/ must actually fail to
compile.
Change-Id: Ic54707eb3bfa19287b4ed52335066fc0fbf19ec1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505397
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This mirrors a lot of the existing matrix ES2 tests, but using
non-square matrices. This is still important because a lot of subtle
bugs can slip through the cracks when rows == columns.
Change-Id: I626c4c2b176c8280da64513d16f59e76e726cbe7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505218
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
GPUs that failed continued to fail when I put in error bars like
`distance(a, b) <= 0.001`, so they're just disabled entirely now.
Presumably their results are very busted.
Change-Id: I0f1b80f661563a20630740f8cfb6ef69f2a47934
Bug: skia:11209, skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503817
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Requires tweaking one inliner test to avoid an Intel driver bug (on
ANGLE).
Bug: chromium:709351
Cq-Do-Not-Cancel-Tryjobs: true
Change-Id: I08fac938396d6b90805ba9650c7a520af888bc12
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503819
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Test "InlinerHonorsGLSLOutParamSemantics" was failing on Wembley devices
and is now disabled on that GPU.
Also, it turns out that the inliner has ignored functions with out
params for a long time now, but our test names haven't been updated to
account for this. So, did some additional cleanup:
- "InlinerHonorsGLSLOutParamSemantics" (the test in question) has been
moved to shared/ and renamed to "OutParamsAreDistinct."
- Removed test "OutParamsNoInline" as it is functionally the same as
"OutParams".
Change-Id: I1431ed197b9216cb482eee4f5e4eb2579a5303f7
Bug: skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502303
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
sk_SecondaryFragColor corresponds to an ES2-only concept
(gl_SecondaryFragColorEXT) and does not have any SPIR-V equivalent.
Two fixes were needed:
- sk_SecondaryFragColor shouldn't be in SPIR-V code at all. Report it as
an error when it appears.
- We don't stop compilation when this error is reported, so we need to
fix up the assertion that the fuzzer initially discovered.
Specifically, the fuzzer found that the `sk_SecondaryFragColor`
variable never got a SPIR-V ID assigned to it in fVariableMap, so the
compiler would assert when assembling an expression containing that
variable. Now, we make sure to populate fVariableMap with an (unused)
ID in `writeGlobalVar` to avoid this crash.
Change-Id: Ib86919dfc9a325b2b82a7f4b2054b747dad7c32f
Bug: oss-fuzz:44096
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501976
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Removed a special case from `writeFunctionCallArgument` which avoided
using a scratch variable for out params; now we always use the scratch
variable and copy it back to the original variable at the end.
Change-Id: I0e446a3fde6d19554943384210bd911f6f9c8cfa
Bug: skia:11052, skia:11919, skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501836
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Upcoming dehydration / rehydration changes require $intLiteral and
$floatLiteral to be present in the symbol table (as all other private
types are). It turns out that even with them marked private, having
them in the symbol table allows them to be incorrectly accessed without
error due to a code path that fails to check for private types.
This CL takes care of that and ultimately results in better output from
PrivateTypes.
Change-Id: Ic47b77a770834079f28c3195545a7cabca8e6cb3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501196
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
GLSL ES2 behavior is explicitly undefined if an out-param is never
written to: "If a function does not write to an out parameter, the value
of the actual parameter is undefined when the function returns."
We do see divergence here in practice: SkVM's behavior (the parameter is
left alone) differs from my GPU's behavior (the parameter is zeroed
out).
SkSL will now report an error if an out parameter is never assigned-to.
There is no control flow analysis performed, so we will not report
cases where the out parameter is assigned-to on some paths but not
others. (Technically the return-on-all-paths logic could be adapted
for this, but it would be a fair amount of work.)
Structs are currently exempt from the rule because custom mesh
specifications require an `out` parameter for a Varyings struct, even if
your mesh program doesn't need Varyings.
Bug: skia:12867
Change-Id: Ie828d3ce91c2c67e008ae304fdb163ffa88d744c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/500440
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously, when attempting to cast a huge value to an int, SkSL would
report an error, then return the IR for
`ScalarCast(Int, FloatLiteral(huge-value))` . Now, to minimize the blast
radius of the error, we report the error but return `IntLiteral(0)`.
We've already reported an error, so there's no need to preserve the
value, and zero is less likely to produce follow-up errors.
(A similar approach is used here and worked well: https://osscs.corp.google.com/skia/skia/+/main:src/sksl/ir/SkSLConstructorCompoundCast.cpp;l=57-59)
Change-Id: Ie8e8d48380cb963466d1f47d123d64e3301cf87c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499563
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The test input was removed at http://review.skia.org/497742.
Change-Id: I7b30f2f70cd0812b900c9c67b70e742b3d96930a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499574
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The test has been moved to shared/, since it's a valid test, but it is
no longer related to inlining, as the inliner no longer attempts to
inline functions with inouts at all.
Also, one function here (outParameterIgnore) actually invoked undefined
behavior and has been removed. According to the GLSL ES2 docs: "If a
function does not write to an out parameter, the value of the actual
parameter is undefined when the function returns." SkVM leaves the value
unchanged, so SKSL_TEST_CPU would pass, but a GPU might clear it (and in
fact, my GPU does).
Change-Id: I77c77ed1354bc980344ec5c406992bd62015f5e5
Bug: skia:11919
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499752
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Now, constant mat+mat, mat-mat, and mat/mat operations can be optimized
away. mat*mat does not operate componentwise and will need to be
handled differently.
Change-Id: Iabac6e58999eac46c256d7dcdb9b95d05de530bc
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498716
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
GLSL supports adding, subtracting, multiplying, and dividing matrices
with scalars. This works by splatting the scalar across every matrix
component and then performing the op componentwise. Our constant folder
now knows how to fold out these simplifications.
Change-Id: Idb8751ec16135e1b61da0d58cfd0505ab31ac087
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497738
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
In a followup CL, these will be updated to properly fold.
Change-Id: I20d125c0d54cbbcf12f7d096beda1fdf75e51b65
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498617
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, matrix-scalar operations did not actually fold, so the tests
didn't live in folding/. In a followup CL, these will fold.
Bug: skia:12819
Change-Id: I6fdacf89088920719e7666d6c9b05ddffaf6cb6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497742
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
SkSL is somehow interpreting a large positive value as a negative one.
Change-Id: I299e0bf389a9fcbfe697741bd33a54df07748753
Bug: skia:12863
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499556
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Some paths through swizzle optimization would replace a swizzles with a
constructor--e.g. `float3(1, 2, 3).y` would be replaced with `float(2)`.
(Constructor::Convert was responsible for replacing this trivial
constructor with the literal `2.0`.)
The optimization code asserted that this replacement would succeed, but
the fuzzer managed to construct a counterexample where the constructor
rejected the value. Specifically, by nesting casts between int3 and
float3, it found a case where Constructor::Convert returned null because
the literal value was out of range for `int` types.
This assertion didn't really add value so removing it was harmless.
Constructor::Convert already reports an error when it fails, and null
returns are handled properly throughout.
Change-Id: I575d441ed90d6b696f6399941c3f6d84698794bc
Bug: oss-fuzz:44045
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499382
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This guards against any potential for conflict with user code.
Change-Id: Iecaf3ead5f8ada50b6dc159a4ad9e7f3e371edc7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499296
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>
This reverts commit a97a6769b5.
Reason for revert: breaking bot Housekeeper-PerCommit-CheckGeneratedFiles -- see http://screen/5FN75fF9tvcQFKR
Based on the diff, I think this just needs to be synced up to latest code and a rebuild should fix it.
Original change's description:
> [skslc] Generate .hlsl test output files
>
> - The build now generates HLSL output when `skia_compile_sksl_tests` is
> enabled.
> - The "blend" and "shared" tests have been enabled for HLSL with the
> exception of 6 tests that exercise intrinsic inverse hyperbolic
> functions, which don't have HLSL equivalents.
>
> Bug: skia:12691, skia:12352
> Change-Id: Ia970f878f75ff58e8e3d47249c2dc2f756c165b4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482778
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: Arman Uguray <armansito@google.com>
> Commit-Queue: Arman Uguray <armansito@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12691, skia:12352
Change-Id: Iaad607d48edd136eee2b60e48c0643b6e90179e9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499216
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>
- The build now generates HLSL output when `skia_compile_sksl_tests` is
enabled.
- The "blend" and "shared" tests have been enabled for HLSL with the
exception of 6 tests that exercise intrinsic inverse hyperbolic
functions, which don't have HLSL equivalents.
Bug: skia:12691, skia:12352
Change-Id: Ia970f878f75ff58e8e3d47249c2dc2f756c165b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482778
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously, any code which emitted a binary expression would always emit
a leading and trailing space. This caused comma expressions to look
goofy: `foo() , bar();` instead of `foo(), bar();`.
Operator::operatorName() now returns the operator token with appropriate
whitespace around it, and tightOperatorName() is a new method which
omits the whitespace. Functions which assemble binary expressions
should now concatenate `x + operatorName() + y` instead of hard-coding
`x + " " + operatorName() + " " + y`. Prefix/postfix expressions should
use `tightOperatorName()` because otherwise negation looks bad (` - 123`
instead of `-123`).
Super low priority, but it was easy to fix.
Change-Id: I3c92832207293a310fb1070b3b5e72455757b0ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497776
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These identifiers are reserved for SkSL internal use (and can't be
exposed to GLSL or Metal anyway).
Change-Id: Id554cbf21ed2fb66785e77700ff79424ecdf66db
Bug: skia:12854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498036
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This eliminates a handful of useless statements. They were harmless, but
presumably this saves optimization work for the GLSL compiler.
(Patterned after http://review.skia.org/496377 )
Change-Id: Ibe135750488a9917b982dcac67f22b3765412ee1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496599
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
These are harmless, but they can cause the Metal compiler to emit a
warning, and these warnings are making some logs messy for Flutter.
Change-Id: I21b7a3c9ae661c55bbbe8bc13d097966180e977d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496377
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
